[Mesa-dev] [Bug 97231] GL_DEPTH_CLAMP doesn't clamp to the far plane
https://bugs.freedesktop.org/show_bug.cgi?id=97231 --- Comment #17 from Jules Blok --- (In reply to Roland Scheidegger from comment #16) > Although I really believe this to be a llvmpipe problem - if other mesa > drivers are affected by it too they probably have their own, different bugs > with it. I think you're right, it looks like the mesa Intel drivers were never able to run these tests correctly. Even when we didn't rely on GL_DEPTH_CLAMP. -- 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
[Mesa-dev] [Bug 97285] Darkness in Dota 2 after Patch "Make Gallium's BlitFramebuffer follow the GL 4.4 sRGB rules"
https://bugs.freedesktop.org/show_bug.cgi?id=97285 --- Comment #6 from Christoph Haag --- If you are unsure whether you are affected, you are not affected. It's *very* dark and you will immediately see it on the startup logo and menu of dota 2 already. Since nobody posted a screenshot or so yet, here is some gameplay of Portal 2 (contains some Portal 2 story spoilers) with the commit in question: https://www.youtube.com/watch?v=Gn0jp0-AABE with the commit reverted: https://www.youtube.com/watch?v=FxIaqZRuWow -- 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] egl/dri2: dri2_make_current: Release previous context's display
On Thu, Aug 11, 2016 at 12:10 AM, Nicolas Boichat wrote: > On Wed, Aug 10, 2016 at 9:44 AM, Michel Dänzer wrote: >> On 10/08/16 03:00 PM, Nicolas Boichat wrote: >>> eglMakeCurrent can also be used to change the active display. In that >>> case, we need to decrement ref_count of the previous display (possibly >>> destroying it), and increment it on the next display. >>> >>> Also, old_dsurf/old_rsurf cannot be non-NULL if old_ctx is NULL, so >>> we only need to test if old_ctx is non-NULL. >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97214 >>> Fixes: 9ee683f877 (egl/dri2: Add reference count for dri2_egl_display) >>> Cc: "12.0" >>> Reported-by: Alexandr Zelinsky >>> Tested-by: Alexandr Zelinsky >>> Signed-off-by: Nicolas Boichat >>> --- >>> src/egl/drivers/dri2/egl_dri2.c | 6 -- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/src/egl/drivers/dri2/egl_dri2.c >>> b/src/egl/drivers/dri2/egl_dri2.c >>> index 3205a36..701e42a 100644 >>> --- a/src/egl/drivers/dri2/egl_dri2.c >>> +++ b/src/egl/drivers/dri2/egl_dri2.c >>> @@ -1285,8 +1285,10 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay >>> *disp, _EGLSurface *dsurf, >>> >>>if (!unbind) >>> dri2_dpy->ref_count++; >>> - if (old_dsurf || old_rsurf || old_ctx) >>> - dri2_display_release(disp); >>> + if (old_ctx) { >>> + EGLDisplay old_disp = >>> _eglGetDisplayHandle(old_ctx->Resource.Display); >>> + dri2_display_release(old_disp); >>> + } >> >> Unfortunately, this change breaks the piglit test "spec@egl >> 1.4@eglterminate then unbind context", because old_ctx != NULL but >> old_ctx->Resource.Display == NULL. Modifying the test to >> >> if (old_ctx && old_ctx->Resource.Display) { >> >> fixes the regression and doesn't seem to cause any other problems. > > This is probably wrong as the display will leak (it definitely should > be freed after calling eglTerminate + eglMakeCurrent(NULL)). > > I think I know where the problem is (the call to > _eglReleaseDisplayResources happens too early), I'm not sure what's > the best solution. I'll have a look. Turns out we destroy old_ctx just above, and then use it again here, fix is simple (swap a few lines). Patch v2 to follow. Thanks! >> Alexandr, does the patch still fix your problem with that modification? >> >> >> Nicolas, this regression is also reproducible with >> LIBGL_ALWAYS_SOFTWARE=1 . Please get used to testing your changes like >> that and only send out changes for review which don't cause any regressions. > > Managed to build piglit, and run it using a locally built mesa. Are > the commands below what you would use? > > export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$MESA_DIR/lib > export LIBGL_DRIVERS_PATH=$MESA_DIR/lib/gallium > export EGL_DRIVERS_PATH=$MESA_DIR/lib > export EGL_LOG_LEVEL=debug > export LIBGL_ALWAYS_SOFTWARE=1 > ./piglit run -p x11_egl -t "spec@egl.*" all results/master I realized that running with --valgrind is quite useful (it's slow, but since there are only 20+ tests, it's fine). ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] egl/dri2: dri2_make_current: Release previous context's display
eglMakeCurrent can also be used to change the active display. In that case, we need to decrement ref_count of the previous display (possibly destroying it), and increment it on the next display. Also, old_dsurf/old_rsurf cannot be non-NULL if old_ctx is NULL, so we only need to test if old_ctx is non-NULL. v2: Save the old display before destroying the context. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97214 Fixes: 9ee683f877 (egl/dri2: Add reference count for dri2_egl_display) Cc: "12.0" Reported-by: Alexandr Zelinsky Tested-by: Alexandr Zelinsky Signed-off-by: Nicolas Boichat --- Alexandr: Can you give this one another try? Thanks! src/egl/drivers/dri2/egl_dri2.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c index 1ad6855..dde9804 100644 --- a/src/egl/drivers/dri2/egl_dri2.c +++ b/src/egl/drivers/dri2/egl_dri2.c @@ -1269,13 +1269,14 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *dsurf, drv->API.DestroySurface(drv, disp, old_dsurf); if (old_rsurf) drv->API.DestroySurface(drv, disp, old_rsurf); - if (old_ctx) - drv->API.DestroyContext(drv, disp, old_ctx); if (!unbind) dri2_dpy->ref_count++; - if (old_dsurf || old_rsurf || old_ctx) - dri2_display_release(disp); + if (old_ctx) { + EGLDisplay old_disp = _eglGetDisplayHandle(old_ctx->Resource.Display); + drv->API.DestroyContext(drv, disp, old_ctx); + dri2_display_release(old_disp); + } return EGL_TRUE; } else { -- 2.8.0.rc3.226.g39d4020 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 97291] Incorrect packing of struct
https://bugs.freedesktop.org/show_bug.cgi?id=97291 Bug ID: 97291 Summary: Incorrect packing of struct Product: Mesa Version: 11.2 Hardware: All OS: Linux (All) Status: NEW Severity: major Priority: medium Component: glsl-compiler Assignee: mesa-dev@lists.freedesktop.org Reporter: dark_syl...@yahoo.com.ar QA Contact: intel-3d-b...@lists.freedesktop.org Created attachment 125694 --> https://bugs.freedesktop.org/attachment.cgi?id=125694&action=edit Shaders with faulty and non faulty versions I'm the main developer of Ogre 2.1 Our shaders produce incorrect results on Mesa. (we always use std140) I believe the problem is caused by incorrect packing of structs. It's 05:30 am now so I couldn't double check yet. I am attaching the Vertex shader and Pixel shader. The main and only difference between the two is that one does shadow mapping, the other one doesn't. The one doing shadow mapping just glitches. By glitches I mean colours are washed out (as if lighting was really wrong), or just black. This happens with all of our samples where shadow mapping is done. I believe this is caused by our unusual struct definition: struct ShadowReceiverData { mat4 texViewProj; vec2 shadowDepthRange; vec4 invShadowMapSize; }; struct Light { vec3 position; vec3 diffuse; vec3 specular; vec3 attenuation; vec3 spotDirection; vec3 spotParams; }; layout(binding = 0) uniform PassBuffer { //Vertex shader (common to both receiver and casters) mat4 viewProj; //Vertex shader mat4 view; ShadowReceiverData shadowRcv[3]; //This is not in the "good" shader //Pixel shader mat3 invViewMatCubemap; float pssmSplitPoints0; //Not present in the "good" shader float pssmSplitPoints1; //Not present in the "good" shader float pssmSplitPoints2; //Not present in the "good" shader Light lights[1]; } pass; glxinfo | grep 'version' server glx version string: 1.4 client glx version string: 1.4 GLX version: 1.4 Max core profile version: 4.1 Max compat profile version: 3.0 Max GLES1 profile version: 1.1 Max GLES[23] profile version: 3.0 OpenGL core profile version string: 4.1 (Core Profile) Mesa 11.2.0 OpenGL core profile shading language version string: 4.10 OpenGL version string: 3.0 Mesa 11.2.0 OpenGL shading language version string: 1.30 OpenGL ES profile version string: OpenGL ES 3.0 Mesa 11.2.0 OpenGL ES profile shading language version string: OpenGL ES GLSL ES 3.00 OpenGL renderer string: Gallium 0.4 on AMD CAPE VERDE (DRM 2.43.0, LLVM 3.8.0) I own a Radeon HD 7770 1GB. May worth noting this bug also happens on SW rasterizer because the exact same output can be seen on a virtual machine. I'm attached the shaders, both under "Good" and "Bad" folder. I'm guessing you're gonna need more information from me so let me know what I'm missing. Ideally I would have time to track this down to the exact variable in the wrong offset but I don't have the time (and it's 05:48am already!). You probably should add our samples to your testing suite. A lot of them don't work with Mesa and would help improve Mesa greatly. Our code can be found at: https://bitbucket.org/sinbad/ogre/src/dc52e186d53611b20d4f99d30266c56c29099ef7/?at=v2-1-pso I can provide assistance compiling our samples. (or binary blobs if it's too much hassle) Just as an example, this is how our PbsSample is supposed to look: http://imgur.com/7lCnZ6h This is how Mesa renders it (incorrectly): http://imgur.com/SfOsBgY This is how Mesa (correctly!) renders it when shadows are disabled (Note: In the pbs sample there are 3 lights but when shadows are disabled Ogre will only use 1 light): http://imgur.com/mS1i74Y In other samples activating shadow mapping just results in zero. My best guess is bad offset calculation. Have fun! -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 97291] Incorrect packing of struct
https://bugs.freedesktop.org/show_bug.cgi?id=97291 --- Comment #1 from Matias N. Goldberg --- Bahahuoasudh Just close it down. I thought I was on latest version but I was not. On Mesa 12.1 this issue has been fixed. Our Compute samples refuse to run, which I guess that's expected. Our Forward3D sample is also not working correctly (it should show hundreds of lights but those lights are gone), which was working correctly in Mesa 11.2 Tomorrow I'll take a look, but I am already seeing incorrect warnings in those shaders: 0:228(28): warning: `nNormal' used uninitialized 0:231(14): warning: `material' used uninitialized Close this ticket I'll create a new one when my mind is fresh -- You are receiving this mail because: 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 00/16] radeonsi: improve handling of temporary arrays
On 10.08.2016 23:36, Marek Olšák wrote: On Wed, Aug 10, 2016 at 9:23 PM, Nicolai Hähnle wrote: Hi, this is a respin of the series which scans the shader's TGSI to determine which channels of an array are actually written to. Most of the st/mesa changes have become unnecessary. Most of the radeon-specific part stays the same. For one F1 2015 shader, it reduces the scratch size from 132096 to 26624 bytes, which is bound to be much nicer on the texture cache. This has been bugging me... is there something we can do to move temporary arrays to registers? F1 2015 is the only game that doesn't "spill VGPRs", yet has the highest scratch usage per shader. (without this series) If a shader uses 32 VGPRs and a *ton* of scratch space, you know something is wrong. We actually already do that partially: in emit_declaration, we check the size of the array, and if it's below a certain threshold (<= 16 currently) it is lowered to LLVM IR that becomes registers. In particular, that one shader has: Before: Shader Stats: SGPRS: 40 VGPRS: 32 Code Size: 3316 LDS: 0 Scratch: 132096 Max Waves: 8 Spilled SGPRs: 0 Spilled VGPRs: 0 After: Shader Stats: SGPRS: 32 VGPRS: 60 Code Size: 3068 LDS: 0 Scratch: 26624 Max Waves: 4 Spilled SGPRs: 0 Spilled VGPRs: 0 Looks like some of the arrays now land in VGPRs since they have become smaller with that series. There are still a _lot_ of weaknesses in all of this, and they mostly have to do with limitations that are rather deeply baked into assumptions of LLVM's codegen architecture. The biggest problem is that an array in VGPRs needs to be represented somehow in the codegen, and it is currently being represented as one of the VGPR vector register classes, which go up to VReg_512, i.e. 16 registers. Two problems with that: 1. The granularity sucks. If you have an array of 10 entries, it'll end up effectively using 16 registers anyway. 2. You can't go above arrays of size 16. (Though to be fair, once you reach that size, you should probably start worrying about VGPR pressure.) Some other issues are that 3. It should really be LLVM that decides how to lower an array, not Mesa. Ideally, LLVM should be able to make an intelligent decision based on the overall register pressure. 4. We currently don't use LDS for shaders. This was disabled because LLVM needs to be taught about interactions with other LDS uses, especially in tessellation. I think fixing point 4 is the thing with the highest impact/effort ratio right now. For point 3, perhaps we could actually extend the alloca lowering even further so that it lowers allocas into VGPRs _after_register_allocation_. But there's a whole can of worms associated with this. (Oh, another thing to keep in mind: we cannot do non-uniform relative indexing of VGPR arrays. This is emulated by a loop in the shader. So depending on the access patterns into arrays, LDS or in extreme cases even scratch space can actually be faster than VGPR.) 1+2 are a serious headache. I'm not deeply enough into all the GlobalISel work going on in LLVM, though I've read some things that make me hopeful that CodeGen based on GlobalISel could help (because it generally makes the process of register assignment more flexible and configurable). Cheers, Nicolai ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 00/16] radeonsi: improve handling of temporary arrays
Am 11.08.2016 um 11:29 schrieb Nicolai Hähnle: On 10.08.2016 23:36, Marek Olšák wrote: On Wed, Aug 10, 2016 at 9:23 PM, Nicolai Hähnle wrote: Hi, this is a respin of the series which scans the shader's TGSI to determine which channels of an array are actually written to. Most of the st/mesa changes have become unnecessary. Most of the radeon-specific part stays the same. For one F1 2015 shader, it reduces the scratch size from 132096 to 26624 bytes, which is bound to be much nicer on the texture cache. This has been bugging me... is there something we can do to move temporary arrays to registers? F1 2015 is the only game that doesn't "spill VGPRs", yet has the highest scratch usage per shader. (without this series) If a shader uses 32 VGPRs and a *ton* of scratch space, you know something is wrong. We actually already do that partially: in emit_declaration, we check the size of the array, and if it's below a certain threshold (<= 16 currently) it is lowered to LLVM IR that becomes registers. In particular, that one shader has: Before: Shader Stats: SGPRS: 40 VGPRS: 32 Code Size: 3316 LDS: 0 Scratch: 132096 Max Waves: 8 Spilled SGPRs: 0 Spilled VGPRs: 0 After: Shader Stats: SGPRS: 32 VGPRS: 60 Code Size: 3068 LDS: 0 Scratch: 26624 Max Waves: 4 Spilled SGPRs: 0 Spilled VGPRs: 0 Looks like some of the arrays now land in VGPRs since they have become smaller with that series. There are still a _lot_ of weaknesses in all of this, and they mostly have to do with limitations that are rather deeply baked into assumptions of LLVM's codegen architecture. The biggest problem is that an array in VGPRs needs to be represented somehow in the codegen, and it is currently being represented as one of the VGPR vector register classes, which go up to VReg_512, i.e. 16 registers. Two problems with that: 1. The granularity sucks. If you have an array of 10 entries, it'll end up effectively using 16 registers anyway. 2. You can't go above arrays of size 16. (Though to be fair, once you reach that size, you should probably start worrying about VGPR pressure.) Some other issues are that 3. It should really be LLVM that decides how to lower an array, not Mesa. Ideally, LLVM should be able to make an intelligent decision based on the overall register pressure. 4. We currently don't use LDS for shaders. This was disabled because LLVM needs to be taught about interactions with other LDS uses, especially in tessellation. I think fixing point 4 is the thing with the highest impact/effort ratio right now. For point 3, perhaps we could actually extend the alloca lowering even further so that it lowers allocas into VGPRs _after_register_allocation_. But there's a whole can of worms associated with this. (Oh, another thing to keep in mind: we cannot do non-uniform relative indexing of VGPR arrays. This is emulated by a loop in the shader. So depending on the access patterns into arrays, LDS or in extreme cases even scratch space can actually be faster than VGPR.) 1+2 are a serious headache. I'm not deeply enough into all the GlobalISel work going on in LLVM, though I've read some things that make me hopeful that CodeGen based on GlobalISel could help (because it generally makes the process of register assignment more flexible and configurable). When I initially implemented the support for arrays in radeonsi one of the fundamental problems was that LLVM couldn't handle anything else than power of two vectors in its instruction selection. I looked a bit into fixing this, but never completed it. Sounds like nobody worked on this since then and yes I completely agree with your points. Regards, Christian. Cheers, 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 v2 00/16] radeonsi: improve handling of temporary arrays
On 11.08.2016 11:44, Christian König wrote: Am 11.08.2016 um 11:29 schrieb Nicolai Hähnle: On 10.08.2016 23:36, Marek Olšák wrote: On Wed, Aug 10, 2016 at 9:23 PM, Nicolai Hähnle wrote: Hi, this is a respin of the series which scans the shader's TGSI to determine which channels of an array are actually written to. Most of the st/mesa changes have become unnecessary. Most of the radeon-specific part stays the same. For one F1 2015 shader, it reduces the scratch size from 132096 to 26624 bytes, which is bound to be much nicer on the texture cache. This has been bugging me... is there something we can do to move temporary arrays to registers? F1 2015 is the only game that doesn't "spill VGPRs", yet has the highest scratch usage per shader. (without this series) If a shader uses 32 VGPRs and a *ton* of scratch space, you know something is wrong. We actually already do that partially: in emit_declaration, we check the size of the array, and if it's below a certain threshold (<= 16 currently) it is lowered to LLVM IR that becomes registers. In particular, that one shader has: Before: Shader Stats: SGPRS: 40 VGPRS: 32 Code Size: 3316 LDS: 0 Scratch: 132096 Max Waves: 8 Spilled SGPRs: 0 Spilled VGPRs: 0 After: Shader Stats: SGPRS: 32 VGPRS: 60 Code Size: 3068 LDS: 0 Scratch: 26624 Max Waves: 4 Spilled SGPRs: 0 Spilled VGPRs: 0 Looks like some of the arrays now land in VGPRs since they have become smaller with that series. There are still a _lot_ of weaknesses in all of this, and they mostly have to do with limitations that are rather deeply baked into assumptions of LLVM's codegen architecture. The biggest problem is that an array in VGPRs needs to be represented somehow in the codegen, and it is currently being represented as one of the VGPR vector register classes, which go up to VReg_512, i.e. 16 registers. Two problems with that: 1. The granularity sucks. If you have an array of 10 entries, it'll end up effectively using 16 registers anyway. 2. You can't go above arrays of size 16. (Though to be fair, once you reach that size, you should probably start worrying about VGPR pressure.) Some other issues are that 3. It should really be LLVM that decides how to lower an array, not Mesa. Ideally, LLVM should be able to make an intelligent decision based on the overall register pressure. 4. We currently don't use LDS for shaders. This was disabled because LLVM needs to be taught about interactions with other LDS uses, especially in tessellation. I think fixing point 4 is the thing with the highest impact/effort ratio right now. For point 3, perhaps we could actually extend the alloca lowering even further so that it lowers allocas into VGPRs _after_register_allocation_. But there's a whole can of worms associated with this. (Oh, another thing to keep in mind: we cannot do non-uniform relative indexing of VGPR arrays. This is emulated by a loop in the shader. So depending on the access patterns into arrays, LDS or in extreme cases even scratch space can actually be faster than VGPR.) 1+2 are a serious headache. I'm not deeply enough into all the GlobalISel work going on in LLVM, though I've read some things that make me hopeful that CodeGen based on GlobalISel could help (because it generally makes the process of register assignment more flexible and configurable). When I initially implemented the support for arrays in radeonsi one of the fundamental problems was that LLVM couldn't handle anything else than power of two vectors in its instruction selection. I looked a bit into fixing this, but never completed it. Sounds like nobody worked on this since then and yes I completely agree with your points. I actually tried to add vec3 at some point since it's not uncommon for sample instructions and vertex shader inputs, but also gave up. We really also eventually need things like vec5 for sparse textures (aka PRT), and I think a vec6 for some sample instruction variants, and so on. The current representation of physical registers in LLVM just isn't well suited for that kind of thing, so it ends up being a surprisingly big refactoring project. Nicolai Regards, Christian. Cheers, 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 2/2] aubinator: Fix the tool to correctly decode the DWords
On Wed, Aug 10, 2016 at 11:18 PM, Kenneth Graunke wrote: > On Wednesday, August 10, 2016 2:22:29 PM PDT Jason Ekstrand wrote: > > On Tue, Aug 9, 2016 at 4:52 PM, Sirisha Gandikota < > > sirisha.gandik...@intel.com> wrote: > > > > > From: Sirisha Gandikota > > > > > > Several fixes have been added as part of this as listed below: > > > > > > 1) Fix the mask and add disassembler handling for STATE_DS, STATE_HS > > > as the mask returned wrong values of the fields. > > > > > > 2) Fix the GEN_TYPE_ADDRESS/GEN_TYPE_OFFSET decoding - the address/ > > > offset were handled the same way as the other fields and that gives > > > the wrong values for the address/offset. > > > > > > 3) Decode nested/recurssive structures - Many packets contain nested > > > structures, ex: 3DSATE_SO_BUFFER, STATE_BASE_ADDRESS, etc contain MOC > > > structures. Previously, the aubinator printed 1 if there was a MOC > > > structure. Now we decode the entire structure and print out its fields. > > > > > > 4) Print out the DWord address along with its hex value - For a better > > > clarity of information, it is helpful to print both the address and > > > hex value of the DWord along with the DWord count. Since the DWord0 > > > contains the instruction code and the instruction length, it is > > > unnecessary to print the decoded values for DWord0. This information > > > is already available from the DWord hex value. > > > > > > 5) Decode the and the corresponding fields in the group- The > > > tag can have fields of several types including structures. A > > > group can contain one or more number of fields and this has be > correctly > > > decoded. Previously, aubinator did not decode the groups or the > > > fields/structures inside them. Now we decode the in the > > > instructions and structures where the fields in it repeat for any > number > > > of times specified. > > > > > > > One comment for now: Usually, when making a bunch of changes like this, > we > > try and split each functional change into its own patch. That way it's > > more clear what's going on. This is brand new code, so a couple giant > > patches may be ok, but having it split up would be nicer. :) Git has > some > > tools that make this less painful than it might look and I'm available to > > help if you'd like. > > > > I'll try and get around to actually playing with it soon. > > --Jason > > Sirisha asked me whether she should split things up, or squash everything > together. I suggested that she may as well squash everything together > for the initial import (but obviously any future changes should follow > the normal procedure). > > My reasoning was: > > - There was already one giant commit from Kristian importing most of > the tool (patch 1 here). I didn't think it was worth time trying > to split that out into an artificial history. And...when developing > a tool from scratch...what parts go where? > > - I figured reviewing the tool as a whole would be easier than a series > of patches that gave an incomplete picture. > > - There's nothing to bisect across or anything that might regress. > > But I did think it was important to preserve authorship information, > so both Kristian and Sirisha got proper credit for their work. > That's fine. I don't care *that* much either way. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/omx/dec/h264: pass default scaling lists in raster format
Leo, could you take a look, test and/or review this? If you don't have time just say so and I'm going to give it a try. Regards, Christian. Am 09.08.2016 um 11:54 schrieb Christian König: From: Indrajit Das --- src/gallium/state_trackers/omx/vid_dec_h264.c | 40 +-- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/gallium/state_trackers/omx/vid_dec_h264.c b/src/gallium/state_trackers/omx/vid_dec_h264.c index bc7feaa..10f2959 100644 --- a/src/gallium/state_trackers/omx/vid_dec_h264.c +++ b/src/gallium/state_trackers/omx/vid_dec_h264.c @@ -50,35 +50,35 @@ struct dpb_list { }; static const uint8_t Default_4x4_Intra[16] = { -6, 13, 13, 20, 20, 20, 28, 28, - 28, 28, 32, 32, 32, 37, 37, 42 +6, 13, 20, 28, 13, 20, 28, 32, + 20, 28, 32, 37, 28, 32, 37, 42 }; static const uint8_t Default_4x4_Inter[16] = { - 10, 14, 14, 20, 20, 20, 24, 24, - 24, 24, 27, 27, 27, 30, 30, 34 + 10, 14, 20, 24, 14, 20, 24, 27, + 20, 24, 27, 30, 24, 27, 30, 34 }; static const uint8_t Default_8x8_Intra[64] = { -6, 10, 10, 13, 11, 13, 16, 16, - 16, 16, 18, 18, 18, 18, 18, 23, - 23, 23, 23, 23, 23, 25, 25, 25, - 25, 25, 25, 25, 27, 27, 27, 27, - 27, 27, 27, 27, 29, 29, 29, 29, - 29, 29, 29, 31, 31, 31, 31, 31, - 31, 33, 33, 33, 33, 33, 36, 36, - 36, 36, 38, 38, 38, 40, 40, 42 +6, 10, 13, 16, 18, 23, 25, 27, + 10, 11, 16, 18, 23, 25, 27, 29, + 13, 16, 18, 23, 25, 27, 29, 31, + 16, 18, 23, 25, 27, 29, 31, 33, + 18, 23, 25, 27, 29, 31, 33, 36, + 23, 25, 27, 29, 31, 33, 36, 38, + 25, 27, 29, 31, 33, 36, 38, 40, + 27, 29, 31, 33, 36, 38, 40, 42 }; static const uint8_t Default_8x8_Inter[64] = { -9, 13, 13, 15, 13, 15, 17, 17, - 17, 17, 19, 19, 19, 19, 19, 21, - 21, 21, 21, 21, 21, 22, 22, 22, - 22, 22, 22, 22, 24, 24, 24, 24, - 24, 24, 24, 24, 25, 25, 25, 25, - 25, 25, 25, 27, 27, 27, 27, 27, - 27, 28, 28, 28, 28, 28, 30, 30, - 30, 30, 32, 32, 32, 33, 33, 35 +9, 13, 15, 17, 19, 21, 22, 24, + 13, 13, 17, 19, 21, 22, 24, 25, + 15, 17, 19, 21, 22, 24, 25, 27, + 17, 19, 21, 22, 24, 25, 27, 28, + 19, 21, 22, 24, 25, 27, 28, 30, + 21, 22, 24, 25, 27, 28, 30, 32, + 22, 24, 25, 27, 28, 30, 32, 33, + 24, 25, 27, 28, 30, 32, 33, 35 }; static void vid_dec_h264_Decode(vid_dec_PrivateType *priv, struct vl_vlc *vlc, unsigned min_bits_left); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 00/16] radeonsi: improve handling of temporary arrays
On Thu, Aug 11, 2016 at 11:29 AM, Nicolai Hähnle wrote: > On 10.08.2016 23:36, Marek Olšák wrote: >> >> On Wed, Aug 10, 2016 at 9:23 PM, Nicolai Hähnle >> wrote: >>> >>> Hi, >>> >>> this is a respin of the series which scans the shader's TGSI to determine >>> which channels of an array are actually written to. Most of the st/mesa >>> changes have become unnecessary. Most of the radeon-specific part stays >>> the same. >>> >>> For one F1 2015 shader, it reduces the scratch size from 132096 to 26624 >>> bytes, which is bound to be much nicer on the texture cache. >> >> >> This has been bugging me... is there something we can do to move >> temporary arrays to registers? >> >> F1 2015 is the only game that doesn't "spill VGPRs", yet has the >> highest scratch usage per shader. (without this series) >> >> If a shader uses 32 VGPRs and a *ton* of scratch space, you know >> something is wrong. > > > We actually already do that partially: in emit_declaration, we check the > size of the array, and if it's below a certain threshold (<= 16 currently) > it is lowered to LLVM IR that becomes registers. In particular, that one > shader has: > > Before: Shader Stats: SGPRS: 40 VGPRS: 32 Code Size: 3316 LDS: 0 Scratch: > 132096 Max Waves: 8 Spilled SGPRs: 0 Spilled VGPRs: 0 > After: Shader Stats: SGPRS: 32 VGPRS: 60 Code Size: 3068 LDS: 0 Scratch: > 26624 Max Waves: 4 Spilled SGPRs: 0 Spilled VGPRs: 0 > > Looks like some of the arrays now land in VGPRs since they have become > smaller with that series. > > There are still a _lot_ of weaknesses in all of this, and they mostly have > to do with limitations that are rather deeply baked into assumptions of > LLVM's codegen architecture. > > The biggest problem is that an array in VGPRs needs to be represented > somehow in the codegen, and it is currently being represented as one of the > VGPR vector register classes, which go up to VReg_512, i.e. 16 registers. > Two problems with that: > > 1. The granularity sucks. If you have an array of 10 entries, it'll end up > effectively using 16 registers anyway. > > 2. You can't go above arrays of size 16. (Though to be fair, once you reach > that size, you should probably start worrying about VGPR pressure.) > > Some other issues are that > > 3. It should really be LLVM that decides how to lower an array, not Mesa. > Ideally, LLVM should be able to make an intelligent decision based on the > overall register pressure. > > 4. We currently don't use LDS for shaders. This was disabled because LLVM > needs to be taught about interactions with other LDS uses, especially in > tessellation. I think we should first focus on PS and CS. Sadly, LDS is pretty small. We can spill at most 128 dwords (256 on CIK/VI) per thread, but all LDS is used at that point and the wave count is 1 per CU (0.25 per SIMD) = worse than scratch. A more conservative approach is to have a maximum of 16 (32 - CIK/VI) dwords of LDS per thread, which should give us 2 waves per SIMD (with zero PS inputs and no DDX/DDY) or 1-2 waves per SIMD depending on the number of PS inputs (but never 2 on all SIMDs). Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 97285] Darkness in Dota 2 after Patch "Make Gallium's BlitFramebuffer follow the GL 4.4 sRGB rules"
https://bugs.freedesktop.org/show_bug.cgi?id=97285 --- Comment #7 from Nicolai Hähnle --- Created attachment 125699 --> https://bugs.freedesktop.org/attachment.cgi?id=125699&action=edit possible fix I seem to have trashed my Steam installation, so I cannot currently test the attached patch with TF2 or Dota2. However, it does fix some Piglit failures I've been seeing, so please try it out. -- 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] Moving amdgpu/addrlib into a git submodule
On 10.08.2016 15:19, Nicolai Hähnle wrote: > It would be nice to have something more concrete to go on, like what are > the problems _really_ :) Actually, I haven't understood who the other users of addrlib actually are - is it just AMDs proprietary driver ? > What pain do submodules give you that wouldn't be even _worse_ with a > separate script for fetching external sources? In the end - from dist perspective - it's the same kind of problem: you still have to do extra steps to get a full source tree, and you'll need to do extra downloads within the build process (unless you mirror/ merge it into your own repo, to create a full tree for the source package) ... in either ways: really ugly. > Look, there are a bunch of different use cases here. Distro packagers; > Mesa contributors and bleeding-edge users; and us (which is slightly > separate because we're not _just_ Mesa contributors but have parts of > the stack elsewhere, which is why this is coming up in the first place). ACK. > Most of the complaints seem to come from distro packaging, though I have > yet to understand what the exact problems are. We need a 100% automatic build process - from the original source. And we need an *easy* way to apply/deapply our local patches ontop of the source tree - or even better: just have a git repo with just the full tree. If upstream doesn't provide that, all the distros out there have to invent their own mechanisms to transform upstream's mess into sth useful - and that needs to support easy rebasing. > For us, in addition to avoiding the same hassles there's the question of > maintainability, and perhaps (with a huge load of optimism) getting the > closed source folks a bit more involved in the fact that there's another > world out here as well. The hassle comes from the fact that source tree's (instead of having libraries) directly shared between projects in the first place. So, the root is having a bad sw architecture. Why can't we just solve the root problem instead of burning our time w/ workarounds ? By the way: just had a quick look into addrlib and found several defines which aren't used anywhere (in mesa): https://github.com/metux/mesa/commit/a38def1c9c6ac09c151399ec1f91b92a5fac3eb0 Maybe there's another consumer for that, but then it would be private to him and therefore should be defined there. --mtx ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 96626] New account request
https://bugs.freedesktop.org/show_bug.cgi?id=96626 Marek Olšák changed: What|Removed |Added QA Contact|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org Component|Drivers/Gallium/radeonsi|Mesa core Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org --- Comment #2 from Marek Olšák --- I approve this. Re-assigning to Mesa core. -- 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
[Mesa-dev] [Bug 97285] Darkness in Dota 2 after Patch "Make Gallium's BlitFramebuffer follow the GL 4.4 sRGB rules"
https://bugs.freedesktop.org/show_bug.cgi?id=97285 --- Comment #8 from network...@rkmail.ru --- The patched fixed brightness issues for me, tested in TF2, CS:Source, Left4Dead2, ETS2. OpenGL renderer string: Gallium 0.4 on AMD TAHITI (DRM 2.45.0 / 4.7.0-4.g89a2ada-default, LLVM 4.0.0) OpenGL core profile version string: 4.3 (Core Profile) Mesa 12.1.0-devel -- 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] shader subroutine per-context uniform support
FTR, I rebased this series on top of current master (minimal changes) and run a full CTS and piglit run with the i965 [gen 8]. The failing CTS now passes and I have not observed any regression nor in CTS neither in piglit. On Tue, 2016-06-07 at 15:25 +1000, Dave Airlie wrote: > So when I implemented shader subroutines, I ignored part of the > spec, because nobody really cares or uses this extension. > > But since CTS has a test for this feature I thought I'd implement it > a bit better now (still not perfect). > > So the spec says that the values in the subroutine uniforms are > stored per context not per program. > > The first patch is enough to pass the CTS test which doesn't do > any rendering checks, it just writes values from multiple contexts > and reads them back. > > The second 4 tests, push the updating of the subroutines into > the constant upload paths of the drivers, so it calls the API > to update the subroutine constants just before it writes the > constants to the hw. This might not be perfect in some threaded > cases, but it's a lot better than nothing, and I don't think > this API will see much use going forward. > > With these and the previous one liner, there is only one > failing test in CTS for subroutines and it is due to program binary > API. > > Dave. > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev -- Br, Andres ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/5] mesa/subroutines: start adding per-context subroutine index support
On Tue, 2016-06-07 at 16:23 +0200, Iago Toral wrote: > On Tue, 2016-06-07 at 15:25 +1000, Dave Airlie wrote: > > From: Dave Airlie > > > > One piece of ARB_shader_subroutine I ignored was the fact that it > > needs to store the subroutine index data per context and not per > > shader program. > > > > There is one CTS test that tests this: > > GL45-CTS.shader_subroutine.multiple_contexts > > > > However the test only does a write to context and readback, > > it never renders using the values, so this is enough to fix the > > test however not enough to do what the spec says. > > > > So with this patch the info is now stored per context, but > > it gets updated into the program at UseProgram and when the > > values are inserted into the context, which won't help if > > a multiple contexts are in use in multiple threads. > > Stray 'a' in the beginning of the line above. > > > > > Signed-off-by: Dave Airlie > > --- > > src/mesa/main/mtypes.h | 10 ++ > > src/mesa/main/pipelineobj.c | 2 +- > > src/mesa/main/shaderapi.c | 75 > > +++-- > > src/mesa/main/shaderapi.h | 3 +- > > 4 files changed, 58 insertions(+), 32 deletions(-) > > > > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h > > index 471d41d..a7ffbf7 100644 > > --- a/src/mesa/main/mtypes.h > > +++ b/src/mesa/main/mtypes.h > > @@ -4307,6 +4307,15 @@ struct gl_atomic_buffer_binding > > }; > > > > /** > > + * Shader subroutines storage > > + */ > > +struct gl_subroutine_index_binding > > +{ > > + int NumIndex; > > + int *IndexPtr; > > +}; > > + > > +/** > > * Mesa rendering context. > > * > > * This is the central context data structure for Mesa. Almost all > > @@ -4544,6 +4553,7 @@ struct gl_context > > */ > > struct gl_image_unit ImageUnits[MAX_IMAGE_UNITS]; > > > > + struct gl_subroutine_index_binding SubroutineIndex[MESA_SHADER_STAGES]; > > /*@}*/ > > > > struct gl_meta_state *Meta; /**< for "meta" operations */ > > diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c > > index 5a46cfe..def2539 100644 > > --- a/src/mesa/main/pipelineobj.c > > +++ b/src/mesa/main/pipelineobj.c > > @@ -469,7 +469,7 @@ _mesa_bind_pipeline(struct gl_context *ctx, > >FLUSH_VERTICES(ctx, _NEW_PROGRAM | _NEW_PROGRAM_CONSTANTS); > > > >for (i = 0; i < MESA_SHADER_STAGES; i++) > > - > > _mesa_shader_program_init_subroutine_defaults(ctx->_Shader->CurrentProgram[i]); > > + _mesa_shader_program_init_subroutine_defaults(ctx, > > ctx->_Shader->CurrentProgram[i]); > > > >if (ctx->Driver.UseProgram) > > ctx->Driver.UseProgram(ctx, NULL); > > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c > > index 9d440a0..818a88d 100644 > > --- a/src/mesa/main/shaderapi.c > > +++ b/src/mesa/main/shaderapi.c > > @@ -65,6 +65,7 @@ > > #define PATH_MAX _MAX_PATH > > #endif > > > > +static void _mesa_shader_write_subroutine_index(struct gl_context *ctx, > > struct gl_shader *sh); > > /** > > * Return mask of GLSL_x flags by examining the MESA_GLSL env var. > > */ > > @@ -1189,7 +1190,7 @@ use_shader_program(struct gl_context *ctx, > > gl_shader_stage stage, > >shProg = NULL; > > > > if (shProg) > > - _mesa_shader_program_init_subroutine_defaults(shProg); > > + _mesa_shader_program_init_subroutine_defaults(ctx, shProg); > > > > if (*target != shProg) { > >/* Program is current, flush it */ > > @@ -2628,27 +2629,15 @@ _mesa_UniformSubroutinesuiv(GLenum shadertype, > > GLsizei count, > > _mesa_error(ctx, GL_INVALID_OPERATION, "%s", api_name); > > return; > > } > > + > > + ctx->SubroutineIndex[sh->Stage].IndexPtr[j] = indices[j]; > > I think this isn't safe: isn't indices[] owned by the caller? Since this is only assigning values, not memory addresses, I think it is safe. > >} > >i += uni_count; > > } while(i < count); > > > > FLUSH_VERTICES(ctx, _NEW_PROGRAM_CONSTANTS); > > - i = 0; > > - do { > > - struct gl_uniform_storage *uni = sh->SubroutineUniformRemapTable[i]; > > - if (uni == NULL) { > > - i++; > > - continue; > > - } > > - > > - int uni_count = uni->array_elements ? uni->array_elements : 1; > > - > > - memcpy(&uni->storage[0], &indices[i], > > - sizeof(GLuint) * uni_count); > > > > - _mesa_propagate_uniforms_to_driver_storage(uni, 0, uni_count); > > - i += uni_count; > > - } while(i < count); > > + _mesa_shader_write_subroutine_index(ctx, sh); > > } > > > > > > @@ -2690,12 +2679,7 @@ _mesa_GetUniformSubroutineuiv(GLenum shadertype, > > GLint location, > >return; > > } > > > > - { > > - struct gl_uniform_storage *uni = > > sh->SubroutineUniformRemapTable[location]; > > - int offset = location - uni->opaque[stage].index; > > - memcpy(params, &uni->storage[offset],
[Mesa-dev] [AppVeyor] mesa master #1912 completed
Build mesa 1912 completed Commit 06b63f1f43 by Jose Fonseca on 8/11/2016 1:11 PM: appveyor: Force Visual Studio 2013 image.\n\nIt seems the default build image is now Visual Studio 2015, and Visual\nStudio 2013 is not installed. Configure your notification preferences ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/omx/dec/h264: pass default scaling lists in raster format
On 08/11/2016 06:54 AM, Christian König wrote: Leo, could you take a look, test and/or review this? Yeah. It fixes the corruption for certain clip, and no regression found for other clips. Patch is: Tested-by: Leo Liu Regards, Leo If you don't have time just say so and I'm going to give it a try. Regards, Christian. Am 09.08.2016 um 11:54 schrieb Christian König: From: Indrajit Das --- src/gallium/state_trackers/omx/vid_dec_h264.c | 40 +-- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/gallium/state_trackers/omx/vid_dec_h264.c b/src/gallium/state_trackers/omx/vid_dec_h264.c index bc7feaa..10f2959 100644 --- a/src/gallium/state_trackers/omx/vid_dec_h264.c +++ b/src/gallium/state_trackers/omx/vid_dec_h264.c @@ -50,35 +50,35 @@ struct dpb_list { }; static const uint8_t Default_4x4_Intra[16] = { -6, 13, 13, 20, 20, 20, 28, 28, - 28, 28, 32, 32, 32, 37, 37, 42 +6, 13, 20, 28, 13, 20, 28, 32, + 20, 28, 32, 37, 28, 32, 37, 42 }; static const uint8_t Default_4x4_Inter[16] = { - 10, 14, 14, 20, 20, 20, 24, 24, - 24, 24, 27, 27, 27, 30, 30, 34 + 10, 14, 20, 24, 14, 20, 24, 27, + 20, 24, 27, 30, 24, 27, 30, 34 }; static const uint8_t Default_8x8_Intra[64] = { -6, 10, 10, 13, 11, 13, 16, 16, - 16, 16, 18, 18, 18, 18, 18, 23, - 23, 23, 23, 23, 23, 25, 25, 25, - 25, 25, 25, 25, 27, 27, 27, 27, - 27, 27, 27, 27, 29, 29, 29, 29, - 29, 29, 29, 31, 31, 31, 31, 31, - 31, 33, 33, 33, 33, 33, 36, 36, - 36, 36, 38, 38, 38, 40, 40, 42 +6, 10, 13, 16, 18, 23, 25, 27, + 10, 11, 16, 18, 23, 25, 27, 29, + 13, 16, 18, 23, 25, 27, 29, 31, + 16, 18, 23, 25, 27, 29, 31, 33, + 18, 23, 25, 27, 29, 31, 33, 36, + 23, 25, 27, 29, 31, 33, 36, 38, + 25, 27, 29, 31, 33, 36, 38, 40, + 27, 29, 31, 33, 36, 38, 40, 42 }; static const uint8_t Default_8x8_Inter[64] = { -9, 13, 13, 15, 13, 15, 17, 17, - 17, 17, 19, 19, 19, 19, 19, 21, - 21, 21, 21, 21, 21, 22, 22, 22, - 22, 22, 22, 22, 24, 24, 24, 24, - 24, 24, 24, 24, 25, 25, 25, 25, - 25, 25, 25, 27, 27, 27, 27, 27, - 27, 28, 28, 28, 28, 28, 30, 30, - 30, 30, 32, 32, 32, 33, 33, 35 +9, 13, 15, 17, 19, 21, 22, 24, + 13, 13, 17, 19, 21, 22, 24, 25, + 15, 17, 19, 21, 22, 24, 25, 27, + 17, 19, 21, 22, 24, 25, 27, 28, + 19, 21, 22, 24, 25, 27, 28, 30, + 21, 22, 24, 25, 27, 28, 30, 32, + 22, 24, 25, 27, 28, 30, 32, 33, + 24, 25, 27, 28, 30, 32, 33, 35 }; static void vid_dec_h264_Decode(vid_dec_PrivateType *priv, struct vl_vlc *vlc, unsigned min_bits_left); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/5] mesa/subroutines: start adding per-context subroutine index support
On Tue, 2016-06-07 at 15:25 +1000, Dave Airlie wrote: [snip] > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h > index 471d41d..a7ffbf7 100644 > --- a/src/mesa/main/mtypes.h > +++ b/src/mesa/main/mtypes.h > @@ -4307,6 +4307,15 @@ struct gl_atomic_buffer_binding > }; > > /** > + * Shader subroutines storage > + */ > +struct gl_subroutine_index_binding > +{ > + int NumIndex; > + int *IndexPtr; > +}; Nitpick: maybe GLuint, instead of int ? [snip] > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c > index 9d440a0..818a88d 100644 > --- a/src/mesa/main/shaderapi.c > +++ b/src/mesa/main/shaderapi.c [snip] > @@ -2803,29 +2787,60 @@ find_compat_subroutine(struct gl_shader *sh, const > struct glsl_type *type) > } > > static void > -_mesa_shader_init_subroutine_defaults(struct gl_shader *sh) > +_mesa_shader_write_subroutine_index(struct gl_context *ctx, > +struct gl_shader *sh) > { > int i, j; > > - for (i = 0; i < sh->NumSubroutineUniformRemapTable; i++) { > + if (sh->NumSubroutineUniformRemapTable == 0) > + return; > + > + i = 0; > + do { >struct gl_uniform_storage *uni = sh->SubroutineUniformRemapTable[i]; >int uni_count; >int val; > > - if (!uni) > + if (!uni) { > + i++; > continue; > + } Nitpick: we could add a new empty line here. >uni_count = uni->array_elements ? uni->array_elements : 1; > - val = find_compat_subroutine(sh, uni->type); > - > - for (j = 0; j < uni_count; j++) > + for (j = 0; j < uni_count; j++) { > + val = ctx->SubroutineIndex[sh->Stage].IndexPtr[i + j]; > memcpy(&uni->storage[j], &val, sizeof(int)); > + } > >_mesa_propagate_uniforms_to_driver_storage(uni, 0, uni_count); > + i += uni_count; > + } while(i < sh->NumSubroutineUniformRemapTable); > +} > + > +static void > +_mesa_shader_init_subroutine_defaults(struct gl_context *ctx, > + struct gl_shader *sh) > +{ > + int i; > + > + if (ctx->SubroutineIndex[sh->Stage].NumIndex != > sh->NumSubroutineUniformRemapTable) { > + ctx->SubroutineIndex[sh->Stage].IndexPtr = > realloc(ctx->SubroutineIndex[sh->Stage].IndexPtr, > sh->NumSubroutineUniformRemapTable * (sizeof(int))); Nitpick: maybe (sizeof(GLuint)) instead ? (Depending on the comment above). > + ctx->SubroutineIndex[sh->Stage].NumIndex = > sh->NumSubroutineUniformRemapTable; > + } > + > + for (i = 0; i < sh->NumSubroutineUniformRemapTable; i++) { > + struct gl_uniform_storage *uni = sh->SubroutineUniformRemapTable[i]; > + > + if (!uni) > + continue; Nitpick: we could add a new empty line here. [snip] As a final nitpick, there are a lot of new really long lines. Maybe we can give some love to the length limit ☺ Other than the nitpicks, this is: Reviewed-by: Andres Gomez -- Br, Andres ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] mesa: add api to write subroutine indicies to the program storage.
On Tue, 2016-06-07 at 15:25 +1000, Dave Airlie wrote: > From: Dave Airlie > > This writes the subroutine indicies to the program storage for > a stage. This API is intended to be used by drivers to update > the uniform storage before uploading to the hw. > > This isn't the most thread safe effort, but it will be significantly > more multi-context safe. > > Signed-off-by: Dave Airlie > --- > src/mesa/main/shaderapi.c | 10 ++ > src/mesa/main/shaderapi.h | 3 +++ > 2 files changed, 13 insertions(+) > > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c > index 818a88d..07c581f 100644 > --- a/src/mesa/main/shaderapi.c > +++ b/src/mesa/main/shaderapi.c > @@ -2816,6 +2816,16 @@ _mesa_shader_write_subroutine_index(struct gl_context > *ctx, > } while(i < sh->NumSubroutineUniformRemapTable); > } > > +void > +_mesa_shader_write_subroutine_indicies(struct gl_context *ctx, > + gl_shader_stage stage) > +{ > + if (ctx->_Shader->CurrentProgram[stage] && > + ctx->_Shader->CurrentProgram[stage]->_LinkedShaders[stage]) > + _mesa_shader_write_subroutine_index(ctx, > + > ctx->_Shader->CurrentProgram[stage]->_LinkedShaders[stage]); Last line, not properly indented. > +} > + > static void > _mesa_shader_init_subroutine_defaults(struct gl_context *ctx, >struct gl_shader *sh) > diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h > index b3de5fa..968cf97 100644 > --- a/src/mesa/main/shaderapi.h > +++ b/src/mesa/main/shaderapi.h > @@ -69,6 +69,9 @@ _mesa_count_active_attribs(struct gl_shader_program > *shProg); > extern size_t > _mesa_longest_attribute_name_length(struct gl_shader_program *shProg); > > +extern void > +_mesa_shader_write_subroutine_indicies(struct gl_context *ctx, > + gl_shader_stage stage); > extern void GLAPIENTRY > _mesa_AttachObjectARB(GLhandleARB, GLhandleARB); > Other than the small nitpick, this is: Reviewed-by: Andres Gomez -- Br, Andres ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/5] i965: use new subroutine index uploader.
On Tue, 2016-06-07 at 15:25 +1000, Dave Airlie wrote: > From: Dave Airlie > > This plugs the subroutine index updates into the i965 backend, > where it loads constants. > > Signed-off-by: Dave Airlie > --- > src/mesa/drivers/dri/i965/brw_gs_surface_state.c | 2 ++ > src/mesa/drivers/dri/i965/brw_tcs_surface_state.c | 2 ++ > src/mesa/drivers/dri/i965/brw_tes_surface_state.c | 2 ++ > src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 3 ++- > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 3 ++- > src/mesa/drivers/dri/i965/gen6_gs_state.c | 2 ++ > src/mesa/drivers/dri/i965/gen6_vs_state.c | 2 ++ > src/mesa/drivers/dri/i965/gen6_wm_state.c | 3 +++ > src/mesa/drivers/dri/i965/gen7_cs_state.c | 3 +++ > src/mesa/drivers/dri/i965/gen7_ds_state.c | 2 ++ > src/mesa/drivers/dri/i965/gen7_hs_state.c | 3 +++ > 11 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c > b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c > index 63b40d1..f8c0b96 100644 > --- a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c > +++ b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c > @@ -23,6 +23,7 @@ > > #include "main/mtypes.h" > #include "program/prog_parameter.h" > +#include "main/shaderapi.h" > > #include "brw_context.h" > #include "brw_state.h" > @@ -49,6 +50,7 @@ brw_upload_gs_pull_constants(struct brw_context *brw) > /* BRW_NEW_GS_PROG_DATA */ > const struct brw_vue_prog_data *prog_data = &brw->gs.prog_data->base; > > + _mesa_shader_write_subroutine_indicies(&brw->ctx, MESA_SHADER_GEOMETRY); > /* _NEW_PROGRAM_CONSTANTS */ > brw_upload_pull_constants(brw, BRW_NEW_GS_CONSTBUF, &gp->program.Base, > stage_state, &prog_data->base); > diff --git a/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c > b/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c > index 164f05f..0b9aa8d 100644 > --- a/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c > +++ b/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c > @@ -23,6 +23,7 @@ > > #include "main/mtypes.h" > #include "program/prog_parameter.h" > +#include "main/shaderapi.h" > > #include "brw_context.h" > #include "brw_state.h" > @@ -49,6 +50,7 @@ brw_upload_tcs_pull_constants(struct brw_context *brw) > /* BRW_NEW_TCS_PROG_DATA */ > const struct brw_stage_prog_data *prog_data = > &brw->tcs.prog_data->base.base; > > + _mesa_shader_write_subroutine_indicies(&brw->ctx, MESA_SHADER_TESS_CTRL); > /* _NEW_PROGRAM_CONSTANTS */ > brw_upload_pull_constants(brw, BRW_NEW_TCS_CONSTBUF, &tcp->program.Base, > stage_state, prog_data); > diff --git a/src/mesa/drivers/dri/i965/brw_tes_surface_state.c > b/src/mesa/drivers/dri/i965/brw_tes_surface_state.c > index 2fa954d..6d94b91 100644 > --- a/src/mesa/drivers/dri/i965/brw_tes_surface_state.c > +++ b/src/mesa/drivers/dri/i965/brw_tes_surface_state.c > @@ -23,6 +23,7 @@ > > #include "main/mtypes.h" > #include "program/prog_parameter.h" > +#include "main/shaderapi.h" > > #include "brw_context.h" > #include "brw_state.h" > @@ -49,6 +50,7 @@ brw_upload_tes_pull_constants(struct brw_context *brw) > /* BRW_NEW_TES_PROG_DATA */ > const struct brw_stage_prog_data *prog_data = > &brw->tes.prog_data->base.base; > > + _mesa_shader_write_subroutine_indicies(&brw->ctx, MESA_SHADER_TESS_EVAL); > /* _NEW_PROGRAM_CONSTANTS */ > brw_upload_pull_constants(brw, BRW_NEW_TES_CONSTBUF, &dp->program.Base, > stage_state, prog_data); > diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c > b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c > index 1036cda..b12c3d0 100644 > --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c > +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c > @@ -31,7 +31,7 @@ > > #include "main/mtypes.h" > #include "program/prog_parameter.h" > - > +#include "main/shaderapi.h" Why the extra empty line? If so, I would put it after the new include. > #include "brw_context.h" > #include "brw_state.h" > #include "intel_buffer_objects.h" > @@ -118,6 +118,7 @@ brw_upload_vs_pull_constants(struct brw_context *brw) > /* BRW_NEW_VS_PROG_DATA */ > const struct brw_stage_prog_data *prog_data = > &brw->vs.prog_data->base.base; > > + _mesa_shader_write_subroutine_indicies(&brw->ctx, MESA_SHADER_VERTEX); > /* _NEW_PROGRAM_CONSTANTS */ > brw_upload_pull_constants(brw, BRW_NEW_VS_CONSTBUF, &vp->program.Base, > stage_state, prog_data); > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > index b73d5d5..651480e 100644 > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > @@ -38,7 +38,7 @@ > #include "program/prog_parameter.h" > #include "program/prog_instruction.h" > #include "main/fra
Re: [Mesa-dev] [PATCH 4/5] st/mesa: use the new subroutine index upload API.
On Tue, 2016-06-07 at 15:25 +1000, Dave Airlie wrote: > From: Dave Airlie > > This plugs the new API into the gallium state tracker. > > Signed-off-by: Dave Airlie > --- > src/mesa/state_tracker/st_atom_constbuf.c | 19 +++ > src/mesa/state_tracker/st_cb_bitmap.c | 2 +- > src/mesa/state_tracker/st_cb_drawpixels.c | 4 ++-- > 3 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/src/mesa/state_tracker/st_atom_constbuf.c > b/src/mesa/state_tracker/st_atom_constbuf.c > index a980dbe..2646c00 100644 > --- a/src/mesa/state_tracker/st_atom_constbuf.c > +++ b/src/mesa/state_tracker/st_atom_constbuf.c > @@ -34,7 +34,7 @@ > #include "main/imports.h" > #include "program/prog_parameter.h" > #include "program/prog_print.h" > - > +#include "main/shaderapi.h" > #include "pipe/p_context.h" > #include "pipe/p_defines.h" > #include "util/u_inlines.h" > @@ -55,8 +55,10 @@ > */ > void st_upload_constants( struct st_context *st, >struct gl_program_parameter_list *params, > - unsigned shader_type) > + gl_shader_stage stage) It would be good to update also the declaration at src/mesa/state_tracker/st_atom_constbuf.h, I think. Since that would mean the inclusion of the shader_enums.h header in that file, I leave the decision to you ... maybe it is better as it is in your patch. > { > + unsigned shader_type = st_shader_stage_to_ptarget(stage); > + > assert(shader_type == PIPE_SHADER_VERTEX || >shader_type == PIPE_SHADER_FRAGMENT || >shader_type == PIPE_SHADER_GEOMETRY || > @@ -92,6 +94,7 @@ void st_upload_constants( struct st_context *st, >if (params->StateFlags) > _mesa_load_state_parameters(st->ctx, params); > > + _mesa_shader_write_subroutine_indicies(st->ctx, stage); >/* We always need to get a new buffer, to keep the drivers simple and > * avoid gratuitous rendering synchronization. > * Let's use a user buffer to avoid an unnecessary copy. > @@ -140,7 +143,7 @@ static void update_vs_constants(struct st_context *st ) > struct st_vertex_program *vp = st->vp; > struct gl_program_parameter_list *params = vp->Base.Base.Parameters; > > - st_upload_constants( st, params, PIPE_SHADER_VERTEX ); > + st_upload_constants( st, params, MESA_SHADER_VERTEX ); > } > > > @@ -163,7 +166,7 @@ static void update_fs_constants(struct st_context *st ) > struct st_fragment_program *fp = st->fp; > struct gl_program_parameter_list *params = fp->Base.Base.Parameters; > > - st_upload_constants( st, params, PIPE_SHADER_FRAGMENT ); > + st_upload_constants( st, params, MESA_SHADER_FRAGMENT ); > } > > > @@ -185,7 +188,7 @@ static void update_gs_constants(struct st_context *st ) > > if (gp) { >params = gp->Base.Base.Parameters; > - st_upload_constants( st, params, PIPE_SHADER_GEOMETRY ); > + st_upload_constants( st, params, MESA_SHADER_GEOMETRY ); > } > } > > @@ -207,7 +210,7 @@ static void update_tcs_constants(struct st_context *st ) > > if (tcp) { >params = tcp->Base.Base.Parameters; > - st_upload_constants( st, params, PIPE_SHADER_TESS_CTRL ); > + st_upload_constants( st, params, MESA_SHADER_TESS_CTRL ); > } > } > > @@ -229,7 +232,7 @@ static void update_tes_constants(struct st_context *st ) > > if (tep) { >params = tep->Base.Base.Parameters; > - st_upload_constants( st, params, PIPE_SHADER_TESS_EVAL ); > + st_upload_constants( st, params, MESA_SHADER_TESS_EVAL ); > } > } > > @@ -251,7 +254,7 @@ static void update_cs_constants(struct st_context *st ) > > if (cp) { >params = cp->Base.Base.Parameters; > - st_upload_constants( st, params, PIPE_SHADER_COMPUTE ); > + st_upload_constants( st, params, MESA_SHADER_COMPUTE ); > } > } > > diff --git a/src/mesa/state_tracker/st_cb_bitmap.c > b/src/mesa/state_tracker/st_cb_bitmap.c > index b4d04b4..d062907 100644 > --- a/src/mesa/state_tracker/st_cb_bitmap.c > +++ b/src/mesa/state_tracker/st_cb_bitmap.c > @@ -212,7 +212,7 @@ setup_render_state(struct gl_context *ctx, >COPY_4V(colorSave, ctx->Current.Attrib[VERT_ATTRIB_COLOR0]); >COPY_4V(ctx->Current.Attrib[VERT_ATTRIB_COLOR0], color); >st_upload_constants(st, st->fp->Base.Base.Parameters, > - PIPE_SHADER_FRAGMENT); > + MESA_SHADER_FRAGMENT); >COPY_4V(ctx->Current.Attrib[VERT_ATTRIB_COLOR0], colorSave); > } > > diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c > b/src/mesa/state_tracker/st_cb_drawpixels.c > index 311ba25..0411156 100644 > --- a/src/mesa/state_tracker/st_cb_drawpixels.c > +++ b/src/mesa/state_tracker/st_cb_drawpixels.c > @@ -1121,7 +1121,7 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y, > * into the constant buffer, we need to update them > */ >st_upload_c
Re: [Mesa-dev] [PATCH 5/5] mesa/subroutines: drop the old subroutine index uploads.
On Tue, 2016-06-07 at 15:25 +1000, Dave Airlie wrote: > From: Dave Airlie > > We used to upload the indices when they changed, now we rely > on the drivers calling the correct hook to have the values > updated from the context storage. > > Signed-off-by: Dave Airlie > --- > src/mesa/main/shaderapi.c | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c > index 07c581f..e6ca7ed 100644 > --- a/src/mesa/main/shaderapi.c > +++ b/src/mesa/main/shaderapi.c > @@ -65,7 +65,6 @@ > #define PATH_MAX _MAX_PATH > #endif > > -static void _mesa_shader_write_subroutine_index(struct gl_context *ctx, > struct gl_shader *sh); > /** > * Return mask of GLSL_x flags by examining the MESA_GLSL env var. > */ > @@ -2554,7 +2553,6 @@ _mesa_GetActiveSubroutineName(GLuint program, GLenum > shadertype, > length, name, api_name); > } > > - > GLvoid GLAPIENTRY > _mesa_UniformSubroutinesuiv(GLenum shadertype, GLsizei count, > const GLuint *indices) > @@ -2636,8 +2634,6 @@ _mesa_UniformSubroutinesuiv(GLenum shadertype, GLsizei > count, > } while(i < count); > > FLUSH_VERTICES(ctx, _NEW_PROGRAM_CONSTANTS); > - > - _mesa_shader_write_subroutine_index(ctx, sh); > } > > > @@ -2844,8 +2840,6 @@ _mesa_shader_init_subroutine_defaults(struct gl_context > *ctx, > continue; >ctx->SubroutineIndex[sh->Stage].IndexPtr[i] = > find_compat_subroutine(sh, uni->type); > } > - > - _mesa_shader_write_subroutine_index(ctx, sh); > } > > void This is: Reviewed-by: Andres Gomez -- Br, Andres ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 97291] Incorrect packing of struct
https://bugs.freedesktop.org/show_bug.cgi?id=97291 Brian Paul changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #2 from Brian Paul --- As you wish. -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 96626] New account request
https://bugs.freedesktop.org/show_bug.cgi?id=96626 Brian Paul changed: What|Removed |Added Product|Mesa|freedesktop.org Version|git |unspecified Assignee|mesa-dev@lists.freedesktop. |sitewranglers@lists.freedes |org |ktop.org Component|Mesa core |New Accounts QA Contact|mesa-dev@lists.freedesktop. | |org | --- Comment #3 from Brian Paul --- Reassigning to fd.o admins. -- 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
[Mesa-dev] [PATCH] st/vdpau: change the order in which filters are applied
Apply the median and matrix filter before the compostioning we apply the deinterlacing first to avoid the extra overhead in processing the past and the future surfaces in deinterlacing. Signed-off-by: Nayan Deshmukh --- src/gallium/state_trackers/vdpau/mixer.c | 23 +-- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/gallium/state_trackers/vdpau/mixer.c b/src/gallium/state_trackers/vdpau/mixer.c index cb0ef03..89453d1 100644 --- a/src/gallium/state_trackers/vdpau/mixer.c +++ b/src/gallium/state_trackers/vdpau/mixer.c @@ -240,8 +240,8 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, struct u_rect rect, clip, *prect, dirty_area; unsigned i, layer = 0; struct pipe_video_buffer *video_buffer; - struct pipe_sampler_view *sampler_view; - struct pipe_surface *surface; + struct pipe_sampler_view *sampler_view, **sampler_views; + struct pipe_surface *surface, **surfaces; vlVdpVideoMixer *vmixer; vlVdpSurface *surf; @@ -325,6 +325,17 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, } } + surfaces = video_buffer->get_surfaces(video_buffer); + sampler_views = video_buffer->get_sampler_view_components(video_buffer); + + if (vmixer->noise_reduction.filter) + vl_median_filter_render(vmixer->noise_reduction.filter, + sampler_views[0], surfaces[0]); + + if (vmixer->sharpness.filter) + vl_matrix_filter_render(vmixer->sharpness.filter, + sampler_views[0], surfaces[0]); + prect = RectToPipe(video_source_rect, &rect); if (!prect) { rect.x0 = 0; @@ -394,14 +405,6 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, else { vl_compositor_render(&vmixer->cstate, compositor, surface, &dirty_area, true); - if (vmixer->noise_reduction.filter) - vl_median_filter_render(vmixer->noise_reduction.filter, - sampler_view, surface); - - if (vmixer->sharpness.filter) - vl_matrix_filter_render(vmixer->sharpness.filter, - sampler_view, surface); - if (vmixer->bicubic.filter) vl_bicubic_filter_render(vmixer->bicubic.filter, sampler_view, dst->surface, -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] vl: add a lanczos interpolation filter v3
Hi Christian, I have a sent a patch which deals with the order in which filter are applied. I will send a patch probably by tomorrow which we use temporary surfaces. Regards, Nayan. On Wed, Aug 10, 2016 at 12:49 AM, Christian König wrote: > Am 09.08.2016 um 19:21 schrieb Nayan Deshmukh: > > Hi Christian, > > A few questions. > > > > On Tue, Aug 9, 2016 at 5:10 PM, Christian König > wrote: >> >> I am more than happy to solve these problems, the Lanczos filtering was >> getting a little stale >> anyway because I was not able to reproduce the problems Andy was facing. >> >> Yeah thought so, the reason is probably that you don't have the necessary >> hardware. >> >> Is that why I need to add a PIPE_BIND_LINEAR to a surface? >> >> Yes exactly. >> >> So I need to use maybe a couple of surfaces alternatively to read and >> write with the filters. This approach should work I guess. >> >> Allocate a temporary surface for each step, apply the necessary filters >> using it and then use the temporary buffer as input for the next step. >> >> See how the deinterlacing filter does this, you should use the same >> approach here. >> >> I would use this order for doing things: >> 1. Median filter for noise reduction. >> 2. Sharpening/blur filter. >> 3. Deinterlacing. >> 4. Compositioning and CC conversion. >> 5. Advanced scaling. >> > > I need to provide the median filter and the blur filter with a sampler view > and the deint filter requires a pipe_video_buffer. I am not sure how to > acheive this. Any suggestions? > > > video buffers are basically just a collection of sampler views and render > targets (up to six). You just need to apply each filter to each plane > separately. > > > Also right now deinterlacing is the first step and the other steps follow. > But if we perform median and sharpening filter before then we also need to > apply them on the past and the future surfaces that we require for > deinterlacing. Am I right? > > > Oh, good point. Might be that we need to change the order to 1) > Deinterlacing, 2) Median 3) Sharpening. > > Otherwise the calculation overhead/memory bandwidth probably start to hit > some limits on low end hardware. > > Regards, > Christian. > > > > Regards, > Nayan. > >> Regards, >> Christian. >> >> >> Am 08.08.2016 um 16:32 schrieb Nayan Deshmukh: >> >> Hi Christian, >> >> I am more than happy to solve these problems, the Lanczos filtering was >> getting a little stale >> anyway because I was not able to reproduce the problems Andy was facing. >> >> On Mon, Aug 8, 2016 at 6:24 PM, Christian König >> wrote: >>> >>> Hi Nayan, >>> >>> ok let's take a step back and put the lanczos filtering aside for a >>> moment. I have another task for you which is more urgent right now. >>> >>> The order we do things in vlVdpVideoMixerRender() was never 100% correct, >>> so we have at least three problems here which needs fixing: >>> >>> 1) The noise reduction and sharpness filter read and write to the same >>> surface at the same time. That only works because we use a linear layout. >>> >> Is that why I need to add a PIPE_BIND_LINEAR to a surface? So I need to >> use maybe a couple of surfaces alternatively to read and write with the >> filters. This approach should work I guess. >> >>> 2) We apply the noise reduction and sharpness filter after the >>> composition. That is rather odd and should be fixed so that we apply those >>> filters on the original video frame instead. >>> >> So we need to apply the filter before the CSC conversion. >>> >>> 3) We use delayed rendering to render into output surfaces directly. We >>> should rather use the DRI3 capabilities to allocate multiple output surfaces >>> instead. >>> >>> Could you take care of some of those issues? Especially #1 has become a >>> problem recently. >>> >> Surely, I will start working on the first 2 problem for now and look at >> the third problem a little later. >> >> Regards, >> Nayan. >> >>> >>> Regards, >>> Christian. >>> >>> >>> Am 04.08.2016 um 19:22 schrieb Nayan Deshmukh: >>> >>> Hi Andy, >>> >>> >>> On Thu, Aug 4, 2016 at 8:48 PM, Andy Furniss wrote: Nayan Deshmukh wrote: > > Hi Andy, > > Thanks for testing my patches. NP >> The scaling happens after CSC. OK, thanks. > I believe there is some misunderstanding here, I was able to see the > artifacts in the video that you sent me previously. But I was not > able to replicate them Yea, I got that - just thought you may want to see how they had changed. > with the pendulum video on my system. Same case this time the > pendulum video plays fine this time too. I am attacing a video of it > here > > > https://drive.google.com/file/d/0B1s62k5GtdBwOVAtOUVaU0V5c1E/view?usp=sharing Hmm, that's interesting for a few reasons. Though my monitor won't do 1366x768 I can replicate the same scale factor windowed with mplayer ... -xy
Re: [Mesa-dev] [PATCH] st/vdpau: change the order in which filters are applied
Am 11.08.2016 um 16:27 schrieb Nayan Deshmukh: Apply the median and matrix filter before the compostioning we apply the deinterlacing first to avoid the extra overhead in processing the past and the future surfaces in deinterlacing. Signed-off-by: Nayan Deshmukh --- src/gallium/state_trackers/vdpau/mixer.c | 23 +-- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/gallium/state_trackers/vdpau/mixer.c b/src/gallium/state_trackers/vdpau/mixer.c index cb0ef03..89453d1 100644 --- a/src/gallium/state_trackers/vdpau/mixer.c +++ b/src/gallium/state_trackers/vdpau/mixer.c @@ -240,8 +240,8 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, struct u_rect rect, clip, *prect, dirty_area; unsigned i, layer = 0; struct pipe_video_buffer *video_buffer; - struct pipe_sampler_view *sampler_view; - struct pipe_surface *surface; + struct pipe_sampler_view *sampler_view, **sampler_views; + struct pipe_surface *surface, **surfaces; vlVdpVideoMixer *vmixer; vlVdpSurface *surf; @@ -325,6 +325,17 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, } } + surfaces = video_buffer->get_surfaces(video_buffer); + sampler_views = video_buffer->get_sampler_view_components(video_buffer); + + if (vmixer->noise_reduction.filter) + vl_median_filter_render(vmixer->noise_reduction.filter, + sampler_views[0], surfaces[0]); + + if (vmixer->sharpness.filter) + vl_matrix_filter_render(vmixer->sharpness.filter, + sampler_views[0], surfaces[0]); + You need to apply the filter to all surfaces, not just the first one. E.g. you need to loop from 0 to VL_MAX_SURFACES and apply the filter to each surface which is present. Regards, Christian. prect = RectToPipe(video_source_rect, &rect); if (!prect) { rect.x0 = 0; @@ -394,14 +405,6 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, else { vl_compositor_render(&vmixer->cstate, compositor, surface, &dirty_area, true); - if (vmixer->noise_reduction.filter) - vl_median_filter_render(vmixer->noise_reduction.filter, - sampler_view, surface); - - if (vmixer->sharpness.filter) - vl_matrix_filter_render(vmixer->sharpness.filter, - sampler_view, surface); - if (vmixer->bicubic.filter) vl_bicubic_filter_render(vmixer->bicubic.filter, sampler_view, dst->surface, ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 97291] Incorrect packing of struct
https://bugs.freedesktop.org/show_bug.cgi?id=97291 --- Comment #3 from Ilia Mirkin --- (In reply to Matias N. Goldberg from comment #1) > Bahahuoasudh > > Just close it down. > I thought I was on latest version but I was not. > > On Mesa 12.1 this issue has been fixed. > > Our Compute samples refuse to run, which I guess that's expected. Compute should work on most recent hardware now. See what's supported where and in which mesa version over at https://people.freedesktop.org/~imirkin/glxinfo/ > Our Forward3D sample is also not working correctly (it should show hundreds > of lights but those lights are gone), which was working correctly in Mesa > 11.2 > > Tomorrow I'll take a look, but I am already seeing incorrect warnings in > those shaders: > 0:228(28): warning: `nNormal' used uninitialized > 0:231(14): warning: `material' used uninitialized Those warnings are broken... they don't handle inout arguments, among likely other things. -- You are receiving this mail because: 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] Question about tesselation shader out varyings and transform feedback
On 05/08/16 12:20, Alejandro Piñeiro wrote: > On 05/08/16 01:53, Timothy Arceri wrote: >> On Thu, 2016-08-04 at 18:36 +0200, Alejandro Piñeiro wrote: >>> Hi, >>> >>> these days I have been trying to fix a test that uses transform >>> feedback >>> on the out varying of a tessellation shader. The relevant part on >>> that >>> shader is like this: >>> >>>layout (vertices=4) out; >>> >>>out block { vec4 value; } user_out[]; >>> >>> The test tries to use block.value as the varying name when calling >>> glTransformFeedbackVaryings, in order to get the data of the 4 >>> vertices. >>> The test fails because on link time, it doesn't find that varying >>> name. >>> >>> On mesa, when linked, mesa tfeedback_candidate_generator (at >>> src/compiler/glsl/link_varyings) adds to the hashmap of possible >>> varyings for transform feedback the following names: block[0].value, >>> block[1].value, block[2].value, block[3].value. If I change the test >>> to >>> use those 4 varyings names, instead of try to get the array directly, >>> the test passes. >>> >>> So now is the moment to justify who is wrong per-spec, if mesa or the >>> test. At this moment Im biased to conclude that the test is wrong. >>> But >>> after reading transform feedback specs (ext, feedback2, feedback3, >>> gl44) >>> and tessellation shader, I was not able to find anything. >>> >>> Could someone (I bet that the best person is Timothy Arceri) guide me >>> a >>> little to know in which part of the spec should I look for? >> For block arrays block[0].value,> block[1].value, block[2].value, >> block[3].value are the correct strings for matching transform feedback >> varyings. I forget which spec states this but its in there somewhere I >> recall looking for it and finding it. > Ok, thanks for confirming. BTW, while looking for info for this email > (see below) I found this paragraph at ARB_separate_shader_objects, that > I think that is what I was searching: > > Tessellation control shader per-vertex output variables and blocks and > tessellation control, tessellation evaluation, and geometry shader > per-vertex input variables and blocks are required to be declared as > arrays, with each element representing input or output values for a single > vertex of a multi-vertex primitive. For the purposes of interface > matching, such variables and blocks are treated as though they were not > declared as arrays. > > > Specifically the last sentence. What do you think? I think that I found a better part of the spec. From GL spec 4.4, section "7.3.1 Program interfaces": "For interfaces involving variables, interface blocks, or subroutines, the entries of active resource lists are generated as follows:" "• For an active interface block declared as an array of instances, separate en- tries will be generated for each active instance. The name of the instance is formed by concatenating the block name, the "[" character, an integer identifying the instance number, and the "]" character." Although I think that it is clear, Timothy, do you mind to confirm if that paragraph justifies properly that the test was wrong, and needs to use block[1].value, block[2].value, etc? Thanks in advance. BR ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/vdpau: change the order in which filters are applied
Apply the median and matrix filter before the compostioning we apply the deinterlacing first to avoid the extra overhead in processing the past and the future surfaces in deinterlacing. Signed-off-by: Nayan Deshmukh --- src/gallium/state_trackers/vdpau/mixer.c | 25 +++-- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/gallium/state_trackers/vdpau/mixer.c b/src/gallium/state_trackers/vdpau/mixer.c index cb0ef03..c280bef 100644 --- a/src/gallium/state_trackers/vdpau/mixer.c +++ b/src/gallium/state_trackers/vdpau/mixer.c @@ -240,8 +240,8 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, struct u_rect rect, clip, *prect, dirty_area; unsigned i, layer = 0; struct pipe_video_buffer *video_buffer; - struct pipe_sampler_view *sampler_view; - struct pipe_surface *surface; + struct pipe_sampler_view *sampler_view, **sampler_views; + struct pipe_surface *surface, **surfaces; vlVdpVideoMixer *vmixer; vlVdpSurface *surf; @@ -325,6 +325,19 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, } } + surfaces = video_buffer->get_surfaces(video_buffer); + sampler_views = video_buffer->get_sampler_view_components(video_buffer); + + for(i = 0; i < VL_NUM_COMPONENTS ; ++i) { + if (vmixer->noise_reduction.filter) + vl_median_filter_render(vmixer->noise_reduction.filter, + sampler_views[i], surfaces[i]); + + if (vmixer->sharpness.filter) + vl_matrix_filter_render(vmixer->sharpness.filter, + sampler_views[i], surfaces[i]); + } + prect = RectToPipe(video_source_rect, &rect); if (!prect) { rect.x0 = 0; @@ -394,14 +407,6 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, else { vl_compositor_render(&vmixer->cstate, compositor, surface, &dirty_area, true); - if (vmixer->noise_reduction.filter) - vl_median_filter_render(vmixer->noise_reduction.filter, - sampler_view, surface); - - if (vmixer->sharpness.filter) - vl_matrix_filter_render(vmixer->sharpness.filter, - sampler_view, surface); - if (vmixer->bicubic.filter) vl_bicubic_filter_render(vmixer->bicubic.filter, sampler_view, dst->surface, -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 97285] Darkness in Dota 2 after Patch "Make Gallium's BlitFramebuffer follow the GL 4.4 sRGB rules"
https://bugs.freedesktop.org/show_bug.cgi?id=97285 --- Comment #9 from Matthew Dawson --- (In reply to Kenneth Graunke from comment #3) > It might also be worth testing with and without multisampling... Oh, I tested this without the patch as well in TF2, and I saw no change enabling or disabling MSAA. I also tested the patch on TF2 and it also fixed the darkness issue, at least on the main screen. I didn't try playing the game. I'm sure I'm affected (the difference between the two are like night and day), but I have also seen it switch when using the affected commmit during game play. -- 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 2/5] mesa: add api to write subroutine indicies to the program storage.
On 11/08/16 15:56, Andres Gomez wrote: > On Tue, 2016-06-07 at 15:25 +1000, Dave Airlie wrote: >> From: Dave Airlie >> >> This writes the subroutine indicies to the program storage for >> a stage. This API is intended to be used by drivers to update >> the uniform storage before uploading to the hw. >> >> This isn't the most thread safe effort, but it will be significantly >> more multi-context safe. >> >> Signed-off-by: Dave Airlie >> --- >> src/mesa/main/shaderapi.c | 10 ++ >> src/mesa/main/shaderapi.h | 3 +++ >> 2 files changed, 13 insertions(+) >> >> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c >> index 818a88d..07c581f 100644 >> --- a/src/mesa/main/shaderapi.c >> +++ b/src/mesa/main/shaderapi.c >> @@ -2816,6 +2816,16 @@ _mesa_shader_write_subroutine_index(struct gl_context >> *ctx, >> } while(i < sh->NumSubroutineUniformRemapTable); >> } >> >> +void >> +_mesa_shader_write_subroutine_indicies(struct gl_context *ctx, Instead of "indicies", do you mean "indices"? >> + gl_shader_stage stage) >> +{ >> + if (ctx->_Shader->CurrentProgram[stage] && >> + ctx->_Shader->CurrentProgram[stage]->_LinkedShaders[stage]) >> + _mesa_shader_write_subroutine_index(ctx, >> + >> ctx->_Shader->CurrentProgram[stage]->_LinkedShaders[stage]); > Last line, not properly indented. > >> +} >> + >> static void >> _mesa_shader_init_subroutine_defaults(struct gl_context *ctx, >>struct gl_shader *sh) >> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h >> index b3de5fa..968cf97 100644 >> --- a/src/mesa/main/shaderapi.h >> +++ b/src/mesa/main/shaderapi.h >> @@ -69,6 +69,9 @@ _mesa_count_active_attribs(struct gl_shader_program >> *shProg); >> extern size_t >> _mesa_longest_attribute_name_length(struct gl_shader_program *shProg); >> >> +extern void >> +_mesa_shader_write_subroutine_indicies(struct gl_context *ctx, >> + gl_shader_stage stage); >> extern void GLAPIENTRY >> _mesa_AttachObjectARB(GLhandleARB, GLhandleARB); >> > Other than the small nitpick, this is: > > Reviewed-by: Andres Gomez > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/5] radeonsi: eliminate PS OUT[1] if dual src blending is off and CB1 is not bound
From: Marek Olšák All VP DX9 ports benefit from this. --- src/gallium/drivers/radeonsi/si_state.c | 11 --- src/gallium/drivers/radeonsi/si_state_shaders.c | 7 +++ 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c index 94dbe4c..5d55448 100644 --- a/src/gallium/drivers/radeonsi/si_state.c +++ b/src/gallium/drivers/radeonsi/si_state.c @@ -2343,31 +2343,20 @@ static void si_set_framebuffer_state(struct pipe_context *ctx, r600_context_add_resource_size(ctx, surf->base.texture); p_atomic_inc(&rtex->framebuffers_bound); if (rtex->dcc_gather_statistics) { /* Dirty tracking must be enabled for DCC usage analysis. */ sctx->framebuffer.compressed_cb_mask |= 1 << i; vi_separate_dcc_start_query(ctx, rtex); } } - /* Set the second SPI format for possible dual-src blending. */ - if (i == 1 && surf) { - sctx->framebuffer.spi_shader_col_format |= - surf->spi_shader_col_format << (i * 4); - sctx->framebuffer.spi_shader_col_format_alpha |= - surf->spi_shader_col_format_alpha << (i * 4); - sctx->framebuffer.spi_shader_col_format_blend |= - surf->spi_shader_col_format_blend << (i * 4); - sctx->framebuffer.spi_shader_col_format_blend_alpha |= - surf->spi_shader_col_format_blend_alpha << (i * 4); - } if (state->zsbuf) { surf = (struct r600_surface*)state->zsbuf; if (!surf->depth_initialized) { si_init_depth_surface(sctx, surf); } r600_context_add_resource_size(ctx, surf->base.texture); } diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c b/src/gallium/drivers/radeonsi/si_state_shaders.c index 87d0b7d..d821397 100644 --- a/src/gallium/drivers/radeonsi/si_state_shaders.c +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c @@ -893,20 +893,27 @@ static inline void si_shader_selector_key(struct pipe_context *ctx, */ key->ps.epilog.spi_shader_col_format = (blend->blend_enable_4bit & blend->need_src_alpha_4bit & sctx->framebuffer.spi_shader_col_format_blend_alpha) | (blend->blend_enable_4bit & ~blend->need_src_alpha_4bit & sctx->framebuffer.spi_shader_col_format_blend) | (~blend->blend_enable_4bit & blend->need_src_alpha_4bit & sctx->framebuffer.spi_shader_col_format_alpha) | (~blend->blend_enable_4bit & ~blend->need_src_alpha_4bit & sctx->framebuffer.spi_shader_col_format); + + /* The output for dual source blending should have +* the same format as the first output. +*/ + if (blend->dual_src_blend) + key->ps.epilog.spi_shader_col_format |= + (key->ps.epilog.spi_shader_col_format & 0xf) << 4; } else key->ps.epilog.spi_shader_col_format = sctx->framebuffer.spi_shader_col_format; /* If alpha-to-coverage is enabled, we have to export alpha * even if there is no color buffer. */ if (!(key->ps.epilog.spi_shader_col_format & 0xf) && blend && blend->alpha_to_coverage) key->ps.epilog.spi_shader_col_format |= V_028710_SPI_SHADER_32_AR; -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/5] gallium/radeon: use unflushed fences for PIPE_QUERY_GPU_FINISHED
From: Marek Olšák --- src/gallium/drivers/radeon/r600_query.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/radeon/r600_query.c b/src/gallium/drivers/radeon/r600_query.c index 592cec1..5825e8b 100644 --- a/src/gallium/drivers/radeon/r600_query.c +++ b/src/gallium/drivers/radeon/r600_query.c @@ -132,21 +132,21 @@ static bool r600_query_sw_begin(struct r600_common_context *rctx, static bool r600_query_sw_end(struct r600_common_context *rctx, struct r600_query *rquery) { struct r600_query_sw *query = (struct r600_query_sw *)rquery; switch(query->b.type) { case PIPE_QUERY_TIMESTAMP_DISJOINT: break; case PIPE_QUERY_GPU_FINISHED: - rctx->b.flush(&rctx->b, &query->fence, 0); + rctx->b.flush(&rctx->b, &query->fence, PIPE_FLUSH_DEFERRED); break; case R600_QUERY_DRAW_CALLS: query->end_result = rctx->num_draw_calls; break; case R600_QUERY_SPILL_DRAW_CALLS: query->end_result = rctx->num_spill_draw_calls; break; case R600_QUERY_COMPUTE_CALLS: query->end_result = rctx->num_compute_calls; break; @@ -208,21 +208,21 @@ static bool r600_query_sw_get_result(struct r600_common_context *rctx, switch (query->b.type) { case PIPE_QUERY_TIMESTAMP_DISJOINT: /* Convert from cycles per millisecond to cycles per second (Hz). */ result->timestamp_disjoint.frequency = (uint64_t)rctx->screen->info.clock_crystal_freq * 1000; result->timestamp_disjoint.disjoint = false; return true; case PIPE_QUERY_GPU_FINISHED: { struct pipe_screen *screen = rctx->b.screen; - result->b = screen->fence_finish(screen, NULL, query->fence, + result->b = screen->fence_finish(screen, &rctx->b, query->fence, wait ? PIPE_TIMEOUT_INFINITE : 0); return result->b; } case R600_QUERY_GPIN_ASIC_ID: result->u32 = 0; return true; case R600_QUERY_GPIN_NUM_SIMD: result->u32 = rctx->screen->info.num_good_compute_units; return true; -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/5] radeonsi: simplify CB_TARGET_MASK logic
From: Marek Olšák we can now rely on CB_COLORn_INFO to disable empty slots. --- src/gallium/drivers/radeonsi/si_state.c | 21 +++-- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c index a44f977..8d9fe53 100644 --- a/src/gallium/drivers/radeonsi/si_state.c +++ b/src/gallium/drivers/radeonsi/si_state.c @@ -81,41 +81,34 @@ static uint32_t S_FIXED(float value, uint32_t frac_bits) /* 12.4 fixed-point */ static unsigned si_pack_float_12p4(float x) { return x <= 0? 0 : x >= 4096 ? 0x : x * 16; } /* * Inferred framebuffer and blender state. * - * One of the reasons CB_TARGET_MASK must be derived from the framebuffer state - * is that: - * - The blend state mask is 0xf most of the time. - * - The COLOR1 format isn't INVALID because of possible dual-source blending, - * so COLOR1 is enabled pretty much all the time. - * So CB_TARGET_MASK is the only register that can disable COLOR1. - * - * Another reason is to avoid a hang with dual source blending. + * CB_TARGET_MASK is emitted here to avoid a hang with dual source blending + * if there is not enough PS outputs. */ static void si_emit_cb_render_state(struct si_context *sctx, struct r600_atom *atom) { struct radeon_winsys_cs *cs = sctx->b.gfx.cs; struct si_state_blend *blend = sctx->queued.named.blend; - uint32_t cb_target_mask = 0, i; - - for (i = 0; i < sctx->framebuffer.state.nr_cbufs; i++) - if (sctx->framebuffer.state.cbufs[i]) - cb_target_mask |= 0xf << (4*i); + uint32_t cb_target_mask, i; + /* CB_COLORn_INFO.FORMAT=INVALID disables empty colorbuffer slots. */ if (blend) - cb_target_mask &= blend->cb_target_mask; + cb_target_mask = blend->cb_target_mask; + else + cb_target_mask = 0x; /* Avoid a hang that happens when dual source blending is enabled * but there is not enough color outputs. This is undefined behavior, * so disable color writes completely. * * Reproducible with Unigine Heaven 4.0 and drirc missing. */ if (blend && blend->dual_src_blend && sctx->ps_shader.cso && (sctx->ps_shader.cso->info.colors_written & 0x3) != 0x3) -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] nir/algebraic: Optimize common array indexing sequence
ping On 07/19/2016 10:37 PM, Ian Romanick wrote: > From: Ian Romanick > > Some shaders include code that looks like: > >uniform int i; >uniform vec4 bones[...]; > >foo(bones[i * 3], bones[i * 3 + 1], bones[i * 3 + 2]); > > CSE would do some work on this: > >x = i * 3 >foo(bones[x], bones[x + 1], bones[x + 2]); > > The compiler may then add '<< 4 + base' to the index calculations. > This results in expressions like > >x = i * 3 >foo(bones[x << 4], bones[(x + 1) << 4], bones[(x + 2) << 4]); > > Just rearranging the math to produce (i * 48) + 16 saves an > instruction, and it allows CSE to do more work. > >x = i * 48; >foo(bones[x], bones[x + 16], bones[x + 32]); > > So, ~6 instructions becomes ~3. > > Some individual shader-db results look pretty bad. However, I have a > really, really hard time believing the change in estimated cycles in, > for example, 3dmmes-taiji/51.shader_test after looking that change in > the generated code. > > G45 > total instructions in shared programs: 4020840 -> 4010070 (-0.27%) > instructions in affected programs: 177460 -> 166690 (-6.07%) > helped: 894 > HURT: 0 > > total cycles in shared programs: 98829000 -> 98784990 (-0.04%) > cycles in affected programs: 3936648 -> 3892638 (-1.12%) > helped: 894 > HURT: 0 > > Ironlake > total instructions in shared programs: 6418887 -> 6408117 (-0.17%) > instructions in affected programs: 177460 -> 166690 (-6.07%) > helped: 894 > HURT: 0 > > total cycles in shared programs: 143504542 -> 143460532 (-0.03%) > cycles in affected programs: 3936648 -> 3892638 (-1.12%) > helped: 894 > HURT: 0 > > Sandy Bridge > total instructions in shared programs: 8357887 -> 8339251 (-0.22%) > instructions in affected programs: 432715 -> 414079 (-4.31%) > helped: 2795 > HURT: 0 > > total cycles in shared programs: 118284184 -> 118207412 (-0.06%) > cycles in affected programs: 6114626 -> 6037854 (-1.26%) > helped: 2478 > HURT: 317 > > Ivy Bridge > total instructions in shared programs: 7669390 -> 7653822 (-0.20%) > instructions in affected programs: 388234 -> 372666 (-4.01%) > helped: 2795 > HURT: 0 > > total cycles in shared programs: 68381982 -> 68263684 (-0.17%) > cycles in affected programs: 1972658 -> 1854360 (-6.00%) > helped: 2458 > HURT: 307 > > Haswell > total instructions in shared programs: 7082636 -> 7067068 (-0.22%) > instructions in affected programs: 388234 -> 372666 (-4.01%) > helped: 2795 > HURT: 0 > > total cycles in shared programs: 68282020 -> 68164158 (-0.17%) > cycles in affected programs: 1891820 -> 1773958 (-6.23%) > helped: 2459 > HURT: 261 > > Broadwell > total instructions in shared programs: 9002466 -> 8985875 (-0.18%) > instructions in affected programs: 658784 -> 642193 (-2.52%) > helped: 2795 > HURT: 5 > > total cycles in shared programs: 78503092 -> 78450404 (-0.07%) > cycles in affected programs: 2873304 -> 2820616 (-1.83%) > helped: 2275 > HURT: 415 > > Skylake > total instructions in shared programs: 9156978 -> 9140387 (-0.18%) > instructions in affected programs: 682625 -> 666034 (-2.43%) > helped: 2795 > HURT: 5 > > total cycles in shared programs: 75591392 -> 75550574 (-0.05%) > cycles in affected programs: 3192120 -> 3151302 (-1.28%) > helped: 2271 > HURT: 425 > > Signed-off-by: Ian Romanick > --- > src/compiler/nir/nir_opt_algebraic.py | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/src/compiler/nir/nir_opt_algebraic.py > b/src/compiler/nir/nir_opt_algebraic.py > index 0f0896b..37cb700 100644 > --- a/src/compiler/nir/nir_opt_algebraic.py > +++ b/src/compiler/nir/nir_opt_algebraic.py > @@ -119,6 +119,17 @@ optimizations = [ > (('~fadd@64', a, ('fmul', c , ('fadd', b, ('fneg', a, > ('flrp', a, b, c), '!options->lower_flrp64'), > (('ffma', a, b, c), ('fadd', ('fmul', a, b), c), 'options->lower_ffma'), > (('~fadd', ('fmul', a, b), c), ('ffma', a, b, c), 'options->fuse_ffma'), > + > + # (a * #b + #c) << #d > + # ((a * #b) << #d) + (#c << #d) > + # (a * (#b << #d)) + (#c << #d) > + (('ishl', ('iadd', ('imul', a, '#b'), '#c'), '#d'), > +('iadd', ('imul', a, ('ishl', b, d)), ('ishl', c, d))), > + > + # (a * #b) << #c > + # a * (#b << #c) > + (('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b, c))), > + > # Comparison simplifications > (('~inot', ('flt', a, b)), ('fge', a, b)), > (('~inot', ('fge', a, b)), ('flt', a, b)), > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/5] radeonsi: don't set CB_COLOR1_INFO for dual src blending
From: Marek Olšák Vulkan doesn't do this. The reason may be that CB_COLOR1_INFO.SOURCE_FORMAT from NI was moved to SPI_SHADER_COL_FORMAT for SI. I asked CB guys about this 2 days ago and they still haven't replied. --- src/gallium/drivers/radeonsi/si_state.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c index 5d55448..a44f977 100644 --- a/src/gallium/drivers/radeonsi/si_state.c +++ b/src/gallium/drivers/radeonsi/si_state.c @@ -2500,27 +2500,20 @@ static void si_emit_framebuffer_state(struct si_context *sctx, struct r600_atom radeon_emit(cs, cb_color_fmask);/* R_028C84_CB_COLOR0_FMASK */ radeon_emit(cs, cb_color_fmask_slice); /* R_028C88_CB_COLOR0_FMASK_SLICE */ radeon_emit(cs, tex->color_clear_value[0]); /* R_028C8C_CB_COLOR0_CLEAR_WORD0 */ radeon_emit(cs, tex->color_clear_value[1]); /* R_028C90_CB_COLOR0_CLEAR_WORD1 */ if (sctx->b.chip_class >= VI) /* R_028C94_CB_COLOR0_DCC_BASE */ radeon_emit(cs, ((!tex->dcc_separate_buffer ? tex->resource.gpu_address : 0) + tex->dcc_offset + tex->surface.level[cb->base.u.tex.level].dcc_offset) >> 8); } - /* set CB_COLOR1_INFO for possible dual-src blending */ - if (i == 1 && state->cbufs[0] && - sctx->framebuffer.dirty_cbufs & (1 << 0)) { - radeon_set_context_reg(cs, R_028C70_CB_COLOR0_INFO + 1 * 0x3C, - cb_color_info); - i++; - } for (; i < 8 ; i++) if (sctx->framebuffer.dirty_cbufs & (1 << i)) radeon_set_context_reg(cs, R_028C70_CB_COLOR0_INFO + i * 0x3C, 0); /* ZS buffer. */ if (state->zsbuf && sctx->framebuffer.dirty_zsbuf) { struct r600_surface *zb = (struct r600_surface*)state->zsbuf; struct r600_texture *rtex = (struct r600_texture*)zb->base.texture; radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 5/5] radeonsi: use current context for DCC feedback-loop decompress, fixes Elemental
From: Marek Olšák This is just a workaround. The problem is described in the code. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96541 --- src/gallium/drivers/radeon/r600_pipe_common.h | 2 +- src/gallium/drivers/radeon/r600_texture.c | 35 ++- src/gallium/drivers/radeonsi/si_blit.c| 12 +++-- src/gallium/drivers/radeonsi/si_descriptors.c | 2 +- 4 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h b/src/gallium/drivers/radeon/r600_pipe_common.h index e4002f9..5375044 100644 --- a/src/gallium/drivers/radeon/r600_pipe_common.h +++ b/src/gallium/drivers/radeon/r600_pipe_common.h @@ -764,21 +764,21 @@ void vi_separate_dcc_stop_query(struct pipe_context *ctx, void vi_separate_dcc_process_and_reset_stats(struct pipe_context *ctx, struct r600_texture *tex); void vi_dcc_clear_level(struct r600_common_context *rctx, struct r600_texture *rtex, unsigned level, unsigned clear_value); void evergreen_do_fast_color_clear(struct r600_common_context *rctx, struct pipe_framebuffer_state *fb, struct r600_atom *fb_state, unsigned *buffers, unsigned *dirty_cbufs, const union pipe_color_union *color); -bool r600_texture_disable_dcc(struct r600_common_screen *rscreen, +bool r600_texture_disable_dcc(struct r600_common_context *rctx, struct r600_texture *rtex); void r600_init_screen_texture_functions(struct r600_common_screen *rscreen); void r600_init_context_texture_functions(struct r600_common_context *rctx); /* r600_viewport.c */ void evergreen_apply_scissor_bug_workaround(struct r600_common_context *rctx, struct pipe_scissor_state *scissor); void r600_set_scissor_enable(struct r600_common_context *rctx, bool enable); void r600_update_vs_writes_viewport_index(struct r600_common_context *rctx, struct tgsi_shader_info *info); diff --git a/src/gallium/drivers/radeon/r600_texture.c b/src/gallium/drivers/radeon/r600_texture.c index ddfa135..78dd177 100644 --- a/src/gallium/drivers/radeon/r600_texture.c +++ b/src/gallium/drivers/radeon/r600_texture.c @@ -393,34 +393,55 @@ static bool r600_texture_discard_dcc(struct r600_common_screen *rscreen, assert(rtex->dcc_separate_buffer == NULL); /* Disable DCC. */ rtex->dcc_offset = 0; /* Notify all contexts about the change. */ r600_dirty_all_framebuffer_states(rscreen); return true; } -bool r600_texture_disable_dcc(struct r600_common_screen *rscreen, +/** + * Disable DCC for the texture. (first decompress, then discard metadata). + * + * Unresolved multi-context synchronization issue: + * + * If context 1 disables DCC and context 2 has queued commands that write + * to the texture via CB with DCC enabled, and the order of operations is + * as follows: + * context 2 queues draw calls rendering to the texture, but doesn't flush + * context 1 disables DCC and flushes + * context 1 & 2 reset descriptors and FB state + * context 2 flushes (new compressed tiles written by the draw calls) + * context 1 & 2 read garbage, because DCC is disabled, yet there are + * compressed tiled + * + * \param rctx the current context if you have one, or rscreen->aux_context + * if you don't. + */ +bool r600_texture_disable_dcc(struct r600_common_context *rctx, struct r600_texture *rtex) { - struct r600_common_context *rctx = - (struct r600_common_context *)rscreen->aux_context; + struct r600_common_screen *rscreen = rctx->screen; if (!r600_can_disable_dcc(rtex)) return false; + if (&rctx->b == rscreen->aux_context) + pipe_mutex_lock(rscreen->aux_context_lock); + /* Decompress DCC. */ - pipe_mutex_lock(rscreen->aux_context_lock); rctx->decompress_dcc(&rctx->b, rtex); rctx->b.flush(&rctx->b, NULL, 0); - pipe_mutex_unlock(rscreen->aux_context_lock); + + if (&rctx->b == rscreen->aux_context) + pipe_mutex_unlock(rscreen->aux_context_lock); return r600_texture_discard_dcc(rscreen, rtex); } static void r600_degrade_tile_mode_to_linear(struct r600_common_context *rctx, struct r600_texture *rtex, bool invalidate_storage) { struct pipe_screen *screen = rctx->b.screen; struct r600_texture *new_tex; @@ -485,39 +506,41 @@ static void r600_degrade_tile_mode_to_linear(struct r600_common_context *rctx, r600_dirty_all_framebuffer_states(rctx->screen); p_atomic_inc(&rctx->screen->dirty_tex_d
Re: [Mesa-dev] [PATCH 2/2] aubinator: Fix the tool to correctly decode the DWords
On Wed, Aug 10, 2016 at 11:18 PM, Kenneth Graunke wrote: > On Wednesday, August 10, 2016 2:22:29 PM PDT Jason Ekstrand wrote: >> On Tue, Aug 9, 2016 at 4:52 PM, Sirisha Gandikota < >> sirisha.gandik...@intel.com> wrote: >> >> > From: Sirisha Gandikota >> > >> > Several fixes have been added as part of this as listed below: >> > >> > 1) Fix the mask and add disassembler handling for STATE_DS, STATE_HS >> > as the mask returned wrong values of the fields. >> > >> > 2) Fix the GEN_TYPE_ADDRESS/GEN_TYPE_OFFSET decoding - the address/ >> > offset were handled the same way as the other fields and that gives >> > the wrong values for the address/offset. >> > >> > 3) Decode nested/recurssive structures - Many packets contain nested >> > structures, ex: 3DSATE_SO_BUFFER, STATE_BASE_ADDRESS, etc contain MOC >> > structures. Previously, the aubinator printed 1 if there was a MOC >> > structure. Now we decode the entire structure and print out its fields. >> > >> > 4) Print out the DWord address along with its hex value - For a better >> > clarity of information, it is helpful to print both the address and >> > hex value of the DWord along with the DWord count. Since the DWord0 >> > contains the instruction code and the instruction length, it is >> > unnecessary to print the decoded values for DWord0. This information >> > is already available from the DWord hex value. >> > >> > 5) Decode the and the corresponding fields in the group- The >> > tag can have fields of several types including structures. A >> > group can contain one or more number of fields and this has be correctly >> > decoded. Previously, aubinator did not decode the groups or the >> > fields/structures inside them. Now we decode the in the >> > instructions and structures where the fields in it repeat for any number >> > of times specified. >> > >> >> One comment for now: Usually, when making a bunch of changes like this, we >> try and split each functional change into its own patch. That way it's >> more clear what's going on. This is brand new code, so a couple giant >> patches may be ok, but having it split up would be nicer. :) Git has some >> tools that make this less painful than it might look and I'm available to >> help if you'd like. >> >> I'll try and get around to actually playing with it soon. >> --Jason > > Sirisha asked me whether she should split things up, or squash everything > together. I suggested that she may as well squash everything together > for the initial import (but obviously any future changes should follow > the normal procedure). > > My reasoning was: > > - There was already one giant commit from Kristian importing most of > the tool (patch 1 here). I didn't think it was worth time trying > to split that out into an artificial history. And...when developing > a tool from scratch...what parts go where? > > - I figured reviewing the tool as a whole would be easier than a series > of patches that gave an incomplete picture. > > - There's nothing to bisect across or anything that might regress. > > But I did think it was important to preserve authorship information, > so both Kristian and Sirisha got proper credit for their work. > > Anyway, that's why it is the way it is. Feel free to blame me for it :) That sounds very reasonable. Sirisha, Thanks for getting the tool ready to send out! Kristian > > ___ > 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 97285] Darkness in Dota 2 after Patch "Make Gallium's BlitFramebuffer follow the GL 4.4 sRGB rules"
https://bugs.freedesktop.org/show_bug.cgi?id=97285 --- Comment #10 from Edmondo Tommasina --- Hi Nicolai Your patch fixes the darkness issue for me. Tested-by: Edmondo Tommasina Thanks! -- 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] glsl: look for frag data bindings with [0] tacked onto the end for arrays
This seems okay... let me run it through our CI first. I should have results later today. I'm going to step out for a bit. On 08/09/2016 05:43 PM, Ilia Mirkin wrote: > ping? do we want this? should i drop it? > > On Wed, Jul 13, 2016 at 3:37 PM, Ilia Mirkin wrote: >> Thanks for confirming, Corentin. >> >> Ian, do you have any opinions on this? Seems like a fairly innocuous >> thing to be doing... >> >> On Fri, Jul 8, 2016 at 3:39 PM, Corentin Wallez wrote: >>> Not sure how reviews work in Mesa, but this patch LGTM. I also tested that >>> it fixes the relevant tests failures it is supposed to address. >>> >>> On Wed, Jul 6, 2016 at 7:40 PM, Ilia Mirkin wrote: The GL spec is very unclear on this point. Apparently this is discussed without resolution in the closed Khronos bugtracker at https://cvs.khronos.org/bugzilla/show_bug.cgi?id=7829 . The recommendation is to allow dropping the [0] for looking up the bindings. The approach taken in this patch is to instead tack on [0]'s for each arrayness level of the output's type, and doing the lookup again. That way, for out vec4 foo[2][2][2] we will end up looking for bindings for foo, foo[0], foo[0][0], and foo[0][0][0], in that order of preference. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96765 Signed-off-by: Ilia Mirkin --- src/compiler/glsl/linker.cpp | 39 --- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index d963f54..9d54c2f 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -2566,6 +2566,7 @@ find_available_slots(unsigned used_mask, unsigned needed_count) /** * Assign locations for either VS inputs or FS outputs * + * \param mem_ctx Temporary ralloc context used for linking * \param prog Shader program whose variables need locations assigned * \param constants Driver specific constant values for the program. * \param target_index Selector for the program target to receive location @@ -2577,7 +2578,8 @@ find_available_slots(unsigned used_mask, unsigned needed_count) * error is emitted to the shader link log and false is returned. */ bool -assign_attribute_or_color_locations(gl_shader_program *prog, +assign_attribute_or_color_locations(void *mem_ctx, +gl_shader_program *prog, struct gl_constants *constants, unsigned target_index) { @@ -2680,16 +2682,31 @@ assign_attribute_or_color_locations(gl_shader_program *prog, } else if (target_index == MESA_SHADER_FRAGMENT) { unsigned binding; unsigned index; + const char *name = var->name; + const glsl_type *type = var->type; + + while (type) { +/* Check if there's a binding for the variable name */ +if (prog->FragDataBindings->get(binding, name)) { + assert(binding >= FRAG_RESULT_DATA0); + var->data.location = binding; + var->data.is_unmatched_generic_inout = 0; + + if (prog->FragDataIndexBindings->get(index, name)) { + var->data.index = index; + } + break; +} -if (prog->FragDataBindings->get(binding, var->name)) { - assert(binding >= FRAG_RESULT_DATA0); - var->data.location = binding; -var->data.is_unmatched_generic_inout = 0; +/* If not, but it's an array type, look for name[0] */ +if (type->is_array()) { + name = ralloc_asprintf(mem_ctx, "%s[0]", name); + type = type->fields.array; + continue; +} - if (prog->FragDataIndexBindings->get(index, var->name)) { - var->data.index = index; - } -} +break; + } } /* From GL4.5 core spec, section 15.2 (Shader Execution): @@ -4816,12 +4833,12 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) prev = i; } - if (!assign_attribute_or_color_locations(prog, &ctx->Const, + if (!assign_attribute_or_color_locations(mem_ctx, prog, &ctx->Const, MESA_SHADER_VERTEX)) { goto done; } - if (!assign_attribute_or_color_locations(prog, &ctx->Const, + if (!assign_attribute_or_color_locations(mem_ctx, prog, &ctx->Const,
[Mesa-dev] [Bug 97285] Darkness in Dota 2 after Patch "Make Gallium's BlitFramebuffer follow the GL 4.4 sRGB rules"
https://bugs.freedesktop.org/show_bug.cgi?id=97285 Vladimir Usikov changed: What|Removed |Added CC||granti...@gmail.com -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] mesa: Add _mesa_enum_to_string2()
The new function is identical to _mesa_enum_to_string(), except that it returns false if the given number is an invalid enum. Bundling validation and lookup into a single function allows us to do both with a single lookup into the enum table. Cc: Haixia Shi Change-Id: I83d9a6e53223d1a971cf8dc22cb34b05b48d7889 --- src/mapi/glapi/gen/gl_enums.py | 20 src/mesa/main/enums.h | 2 ++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/mapi/glapi/gen/gl_enums.py b/src/mapi/glapi/gen/gl_enums.py index 4fc43ab..af21aaf 100644 --- a/src/mapi/glapi/gen/gl_enums.py +++ b/src/mapi/glapi/gen/gl_enums.py @@ -89,6 +89,17 @@ static char token_tmp[20]; */ const char *_mesa_enum_to_string( int nr ) { + const char *str; + _mesa_enum_to_string2(nr, &str); + return str; +} + +/** + * Identical to _mesa_enum_to_string(), except that this function returns false + * when the number is an invalid enum. + */ +bool _mesa_enum_to_string2(int nr, const char **str) +{ enum_elt *elt; elt = bsearch(& nr, enum_string_table_offsets, @@ -97,13 +108,14 @@ const char *_mesa_enum_to_string( int nr ) (cfunc) compar_nr); if (elt != NULL) { - return &enum_string_table[elt->offset]; - } - else { + *str = &enum_string_table[elt->offset]; + return true; + } else { /* this is not re-entrant safe, no big deal here */ _mesa_snprintf(token_tmp, sizeof(token_tmp) - 1, "0x%x", nr); token_tmp[sizeof(token_tmp) - 1] = '\\0'; - return token_tmp; + *str = token_tmp; + return false; } } diff --git a/src/mesa/main/enums.h b/src/mesa/main/enums.h index 0e18cd4..ae804d6 100644 --- a/src/mesa/main/enums.h +++ b/src/mesa/main/enums.h @@ -36,6 +36,7 @@ #ifndef _ENUMS_H_ #define _ENUMS_H_ +#include #ifdef __cplusplus extern "C" { @@ -43,6 +44,7 @@ extern "C" { extern const char *_mesa_enum_to_string( int nr ); +bool _mesa_enum_to_string2(int nr, const char **str); /* Get the name of an enum given that it is a primitive type. Avoids * GL_FALSE/GL_POINTS ambiguity and others. -- 2.9.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/3] mesa: Fix a dEQP test
Fixes test dEQP-GLES3.functional.negative_api.buffer.framebuffer_texture2d. I tested this patch for regressions on Skylake with: % ./deqp-gles3 --deqp-case= 'dEQP-GLES3.functional.negative_api.*' % ./deqp-gles3 --deqp-case='dEQP-GLES3.functional.fbo.color.texcube.*' This patch series lives at git://git.kiwitree.net/~chadv/mesa branch review/fix-deqp-framebuffertexture-error Chad Versace (3): mesa: Document that _mesa_enum_to_string() returns non-null mesa: Add _mesa_enum_to_string2() mesa: Fix glFramebufferTexture* error codes src/mapi/glapi/gen/gl_enums.py | 24 src/mesa/main/enums.h | 2 ++ src/mesa/main/fbobject.c | 32 +--- 3 files changed, 43 insertions(+), 15 deletions(-) -- 2.9.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] mesa: Fix glFramebufferTexture* error codes
If check_textarget() determined that textarget was incorrect, it emitted GL_INVALID_OPERATION. This is the correct behavior when textarget is a valid GLenum but an invalid parameter to the current variant of glFramebufferTexture*(). However, when textarget is not a GLenum at all, then the GL spec requires that GL_INVALID_ENUM be emitted. Fixes test dEQP-GLES3.functional.negative_api.buffer.framebuffer_texture2d. Cc: Haixia Shi Change-Id: I86c492f228720ec8cf9939e741cfc99a5d9fa1bc --- src/mesa/main/fbobject.c | 32 +--- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index 2c01526..3360940 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -3029,20 +3029,30 @@ check_textarget(struct gl_context *ctx, int dims, GLenum target, err = true; } - if (err) { - _mesa_error(ctx, GL_INVALID_OPERATION, - "%s(invalid textarget %s)", - caller, _mesa_enum_to_string(textarget)); - return false; + if (!err) { + /* Make sure textarget is consistent with the texture's type */ + if (target == GL_TEXTURE_CUBE_MAP) { + err = !_mesa_is_cube_face(textarget); + } else { + err = (target != textarget); + } } - /* Make sure textarget is consistent with the texture's type */ - err = (target == GL_TEXTURE_CUBE_MAP) ? - !_mesa_is_cube_face(textarget): (target != textarget); - if (err) { - _mesa_error(ctx, GL_INVALID_OPERATION, - "%s(mismatched texture target)", caller); + const char *enum_str; + + if (_mesa_enum_to_string2(textarget, &enum_str)) { + /* The textarget is a valid GLenum, but is an invalid parameter to + * this variant of glFramebufferTexture*(). + */ + _mesa_error(ctx, GL_INVALID_OPERATION, + "%s(mismatched texture target %s)", caller, enum_str); + } else { + /* The textarget is not a GLenum value. */ + _mesa_error(ctx, GL_INVALID_ENUM, + "%s(invalid textarget %s)", caller, enum_str); + } + return false; } -- 2.9.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] mesa: Document that _mesa_enum_to_string() returns non-null
It always returns non-null, even if the number is an invalid enum. Cc: Haixia Shi Change-Id: I26e8843c96130be972e66f48a49e362442e1bf97 --- src/mapi/glapi/gen/gl_enums.py | 4 1 file changed, 4 insertions(+) diff --git a/src/mapi/glapi/gen/gl_enums.py b/src/mapi/glapi/gen/gl_enums.py index 62cc1b3..4fc43ab 100644 --- a/src/mapi/glapi/gen/gl_enums.py +++ b/src/mapi/glapi/gen/gl_enums.py @@ -83,6 +83,10 @@ static int compar_nr( const int *a, enum_elt *b ) static char token_tmp[20]; +/** + * This function always returns a string. If the number is a valid enum, it + * returns the enum name. Otherwise, it returns a numeric string. + */ const char *_mesa_enum_to_string( int nr ) { enum_elt *elt; -- 2.9.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Document that _mesa_enum_to_string() returns non-null
On 08/11/2016 10:11 AM, Chad Versace wrote: > It always returns non-null, even if the number is an invalid enum. > > Cc: Haixia Shi > Change-Id: I26e8843c96130be972e66f48a49e362442e1bf97 > --- > src/mapi/glapi/gen/gl_enums.py | 4 > 1 file changed, 4 insertions(+) > > diff --git a/src/mapi/glapi/gen/gl_enums.py b/src/mapi/glapi/gen/gl_enums.py > index 62cc1b3..4fc43ab 100644 > --- a/src/mapi/glapi/gen/gl_enums.py > +++ b/src/mapi/glapi/gen/gl_enums.py > @@ -83,6 +83,10 @@ static int compar_nr( const int *a, enum_elt *b ) > > static char token_tmp[20]; > > +/** > + * This function always returns a string. If the number is a valid enum, it > + * returns the enum name. Otherwise, it returns a numeric string. > + */ > const char *_mesa_enum_to_string( int nr ) Maybe take this opportunity to fix the formatting of the function. const char * _mesa_enum_to_string(int nr) > { > enum_elt *elt; > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] anv: pipeline: gen7: fix assert in debug mode
SampleMask is only 8bits long on gen7. Signed-off-by: Lionel Landwerlin Cc: Jason Ekstrand --- src/intel/vulkan/genX_pipeline_util.h | 4 1 file changed, 4 insertions(+) diff --git a/src/intel/vulkan/genX_pipeline_util.h b/src/intel/vulkan/genX_pipeline_util.h index 64b89cd..d56eeb8 100644 --- a/src/intel/vulkan/genX_pipeline_util.h +++ b/src/intel/vulkan/genX_pipeline_util.h @@ -462,7 +462,11 @@ emit_ms_state(struct anv_pipeline *pipeline, * * 3DSTATE_SAMPLE_MASK.SampleMask is 16 bits. */ +#if GEN_GEN >= 8 uint32_t sample_mask = 0x; +#else + uint32_t sample_mask = 0xff; +#endif if (info) { samples = info->rasterizationSamples; -- 2.8.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] anv: GetDeviceImageFormatProperties: fix TRANSFER formats
We let the user believe we support some transfer formats which we don't. This can lead to crashes when actually trying to use those formats for example on dEQP-VK.api.copy_and_blit.image_to_image.* tests. Let all formats we can render to or sample from as meta implements transfers using attachments. Signed-off-by: Lionel Landwerlin Cc: Jason Ekstrand --- src/intel/vulkan/anv_formats.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c index b26e48a..13bde90 100644 --- a/src/intel/vulkan/anv_formats.c +++ b/src/intel/vulkan/anv_formats.c @@ -509,23 +509,21 @@ VkResult anv_GetPhysicalDeviceImageFormatProperties( if (usage & VK_IMAGE_USAGE_TRANSFER_SRC_BIT) { /* Meta implements transfers by sampling from the source image. */ - if (!(format_feature_flags & VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT)) { + if (!(format_feature_flags & (VK_FORMAT_FEATURE_COLOR_ATTACHMENT_BIT | + VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT | +VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT))) { goto unsupported; } } -#if 0 if (usage & VK_IMAGE_USAGE_TRANSFER_DST_BIT) { - if (anv_format_for_vk_format(format)->has_stencil) { - /* Not yet implemented because copying to a W-tiled surface is crazy - * hard. - */ - anv_finishme("support VK_IMAGE_USAGE_TRANSFER_DST_BIT for " - "stencil format"); + /* Meta implements transfers by rendering to the attachment image. */ + if (!(format_feature_flags & (VK_FORMAT_FEATURE_COLOR_ATTACHMENT_BIT | + VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT | +VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT))) { goto unsupported; } } -#endif if (usage & VK_IMAGE_USAGE_SAMPLED_BIT) { if (!(format_feature_flags & VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT)) { -- 2.8.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] mesa: Fix glFramebufferTexture* error codes
On 08/11/2016 10:11 AM, Chad Versace wrote: > If check_textarget() determined that textarget was incorrect, it emitted > GL_INVALID_OPERATION. This is the correct behavior when textarget is > a valid GLenum but an invalid parameter to the current variant of > glFramebufferTexture*(). > > However, when textarget is not a GLenum at all, then the GL spec > requires that GL_INVALID_ENUM be emitted. > > Fixes test dEQP-GLES3.functional.negative_api.buffer.framebuffer_texture2d. > > Cc: Haixia Shi > Change-Id: I86c492f228720ec8cf9939e741cfc99a5d9fa1bc > --- > src/mesa/main/fbobject.c | 32 +--- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c > index 2c01526..3360940 100644 > --- a/src/mesa/main/fbobject.c > +++ b/src/mesa/main/fbobject.c > @@ -3029,20 +3029,30 @@ check_textarget(struct gl_context *ctx, int dims, > GLenum target, >err = true; > } > > - if (err) { > - _mesa_error(ctx, GL_INVALID_OPERATION, > - "%s(invalid textarget %s)", > - caller, _mesa_enum_to_string(textarget)); > - return false; > + if (!err) { > + /* Make sure textarget is consistent with the texture's type */ > + if (target == GL_TEXTURE_CUBE_MAP) { > + err = !_mesa_is_cube_face(textarget); > + } else { > + err = (target != textarget); > + } > } > > - /* Make sure textarget is consistent with the texture's type */ > - err = (target == GL_TEXTURE_CUBE_MAP) ? > - !_mesa_is_cube_face(textarget): (target != textarget); > - > if (err) { > - _mesa_error(ctx, GL_INVALID_OPERATION, > - "%s(mismatched texture target)", caller); > + const char *enum_str; > + > + if (_mesa_enum_to_string2(textarget, &enum_str)) { I'm not sure this is sufficient. What if I pass GL_RED? I'm reasonably certain that should generate GL_INVALID_ENUM, but it will generate GL_INVALID_OPERATION. > + /* The textarget is a valid GLenum, but is an invalid parameter to > + * this variant of glFramebufferTexture*(). > + */ > + _mesa_error(ctx, GL_INVALID_OPERATION, > + "%s(mismatched texture target %s)", caller, enum_str); > + } else { > + /* The textarget is not a GLenum value. */ > + _mesa_error(ctx, GL_INVALID_ENUM, > + "%s(invalid textarget %s)", caller, enum_str); > + } > + >return false; > } > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Document that _mesa_enum_to_string() returns non-null
On 08/11/2016 10:22 AM, Ian Romanick wrote: > On 08/11/2016 10:11 AM, Chad Versace wrote: >> It always returns non-null, even if the number is an invalid enum. >> >> Cc: Haixia Shi >> Change-Id: I26e8843c96130be972e66f48a49e362442e1bf97 >> --- >> src/mapi/glapi/gen/gl_enums.py | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/src/mapi/glapi/gen/gl_enums.py b/src/mapi/glapi/gen/gl_enums.py >> index 62cc1b3..4fc43ab 100644 >> --- a/src/mapi/glapi/gen/gl_enums.py >> +++ b/src/mapi/glapi/gen/gl_enums.py >> @@ -83,6 +83,10 @@ static int compar_nr( const int *a, enum_elt *b ) >> >> static char token_tmp[20]; >> >> +/** >> + * This function always returns a string. If the number is a valid enum, it >> + * returns the enum name. Otherwise, it returns a numeric string. >> + */ >> const char *_mesa_enum_to_string( int nr ) > > Maybe take this opportunity to fix the formatting of the function. > > const char * > _mesa_enum_to_string(int nr) With or without this change, this patch is Reviewed-by: Ian Romanick The R-b for the other two is contingent on being sure patch 3 is actually sufficient. >> { >> enum_elt *elt; >> > > ___ > 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 3/3] mesa: Fix glFramebufferTexture* error codes
On Thu, Aug 11, 2016 at 1:11 PM, Chad Versace wrote: > If check_textarget() determined that textarget was incorrect, it emitted > GL_INVALID_OPERATION. This is the correct behavior when textarget is > a valid GLenum but an invalid parameter to the current variant of > glFramebufferTexture*(). > > However, when textarget is not a GLenum at all, then the GL spec > requires that GL_INVALID_ENUM be emitted. > > Fixes test dEQP-GLES3.functional.negative_api.buffer.framebuffer_texture2d. > > Cc: Haixia Shi > Change-Id: I86c492f228720ec8cf9939e741cfc99a5d9fa1bc > --- > src/mesa/main/fbobject.c | 32 +--- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c > index 2c01526..3360940 100644 > --- a/src/mesa/main/fbobject.c > +++ b/src/mesa/main/fbobject.c > @@ -3029,20 +3029,30 @@ check_textarget(struct gl_context *ctx, int dims, > GLenum target, >err = true; > } > > - if (err) { > - _mesa_error(ctx, GL_INVALID_OPERATION, > - "%s(invalid textarget %s)", > - caller, _mesa_enum_to_string(textarget)); > - return false; > + if (!err) { > + /* Make sure textarget is consistent with the texture's type */ > + if (target == GL_TEXTURE_CUBE_MAP) { > + err = !_mesa_is_cube_face(textarget); > + } else { > + err = (target != textarget); > + } > } > > - /* Make sure textarget is consistent with the texture's type */ > - err = (target == GL_TEXTURE_CUBE_MAP) ? > - !_mesa_is_cube_face(textarget): (target != textarget); > - > if (err) { > - _mesa_error(ctx, GL_INVALID_OPERATION, > - "%s(mismatched texture target)", caller); > + const char *enum_str; > + > + if (_mesa_enum_to_string2(textarget, &enum_str)) { so ... if I feed it, say, GL_CLIP_PLANE0 then it's INVALID_OPERATION but if I feed it 0x then it's INVALID_ENUM? Probably should just list out the valid enums here. I wouldn't be surprised if such a list already existed elsewhere in the file. > + /* The textarget is a valid GLenum, but is an invalid parameter to > + * this variant of glFramebufferTexture*(). > + */ > + _mesa_error(ctx, GL_INVALID_OPERATION, > + "%s(mismatched texture target %s)", caller, enum_str); > + } else { > + /* The textarget is not a GLenum value. */ > + _mesa_error(ctx, GL_INVALID_ENUM, > + "%s(invalid textarget %s)", caller, enum_str); > + } > + >return false; > } > > -- > 2.9.2 > > ___ > 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 2/3] mesa: Add _mesa_enum_to_string2()
On 08/11/2016 10:11 AM, Chad Versace wrote: > The new function is identical to _mesa_enum_to_string(), except that it > returns false if the given number is an invalid enum. Bundling > validation and lookup into a single function allows us to do both with > a single lookup into the enum table. > > Cc: Haixia Shi > Change-Id: I83d9a6e53223d1a971cf8dc22cb34b05b48d7889 > --- > src/mapi/glapi/gen/gl_enums.py | 20 > src/mesa/main/enums.h | 2 ++ > 2 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/src/mapi/glapi/gen/gl_enums.py b/src/mapi/glapi/gen/gl_enums.py > index 4fc43ab..af21aaf 100644 > --- a/src/mapi/glapi/gen/gl_enums.py > +++ b/src/mapi/glapi/gen/gl_enums.py > @@ -89,6 +89,17 @@ static char token_tmp[20]; > */ > const char *_mesa_enum_to_string( int nr ) > { > + const char *str; > + _mesa_enum_to_string2(nr, &str); > + return str; > +} > + > +/** > + * Identical to _mesa_enum_to_string(), except that this function returns > false > + * when the number is an invalid enum. > + */ > +bool _mesa_enum_to_string2(int nr, const char **str) > +{ > enum_elt *elt; > Add an assert(str != NULL); and maybe document that in the function header. > elt = bsearch(& nr, enum_string_table_offsets, > @@ -97,13 +108,14 @@ const char *_mesa_enum_to_string( int nr ) > (cfunc) compar_nr); > > if (elt != NULL) { > - return &enum_string_table[elt->offset]; > - } > - else { > + *str = &enum_string_table[elt->offset]; > + return true; > + } else { >/* this is not re-entrant safe, no big deal here */ >_mesa_snprintf(token_tmp, sizeof(token_tmp) - 1, "0x%x", nr); >token_tmp[sizeof(token_tmp) - 1] = '\\0'; > - return token_tmp; > + *str = token_tmp; > + return false; > } > } > > diff --git a/src/mesa/main/enums.h b/src/mesa/main/enums.h > index 0e18cd4..ae804d6 100644 > --- a/src/mesa/main/enums.h > +++ b/src/mesa/main/enums.h > @@ -36,6 +36,7 @@ > #ifndef _ENUMS_H_ > #define _ENUMS_H_ > > +#include > > #ifdef __cplusplus > extern "C" { > @@ -43,6 +44,7 @@ extern "C" { > > > extern const char *_mesa_enum_to_string( int nr ); > +bool _mesa_enum_to_string2(int nr, const char **str); > > /* Get the name of an enum given that it is a primitive type. Avoids > * GL_FALSE/GL_POINTS ambiguity and others. > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Document that _mesa_enum_to_string() returns non-null
On 08/11/2016 10:30 AM, Ian Romanick wrote: On 08/11/2016 10:22 AM, Ian Romanick wrote: On 08/11/2016 10:11 AM, Chad Versace wrote: It always returns non-null, even if the number is an invalid enum. Cc: Haixia Shi Change-Id: I26e8843c96130be972e66f48a49e362442e1bf97 --- src/mapi/glapi/gen/gl_enums.py | 4 1 file changed, 4 insertions(+) diff --git a/src/mapi/glapi/gen/gl_enums.py b/src/mapi/glapi/gen/gl_enums.py index 62cc1b3..4fc43ab 100644 --- a/src/mapi/glapi/gen/gl_enums.py +++ b/src/mapi/glapi/gen/gl_enums.py @@ -83,6 +83,10 @@ static int compar_nr( const int *a, enum_elt *b ) static char token_tmp[20]; +/** + * This function always returns a string. If the number is a valid enum, it + * returns the enum name. Otherwise, it returns a numeric string. + */ const char *_mesa_enum_to_string( int nr ) Maybe take this opportunity to fix the formatting of the function. I'll fix it before pushing. const char * _mesa_enum_to_string(int nr) With or without this change, this patch is Reviewed-by: Ian Romanick The R-b for the other two is contingent on being sure patch 3 is actually sufficient. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: look for frag data bindings with [0] tacked onto the end for arrays
Or not. :( The patch no longer applies, and I wasn't able to trivially make it apply. I suspect one of Timothy's recent patches is to blame. On 08/11/2016 10:06 AM, Ian Romanick wrote: > This seems okay... let me run it through our CI first. I should have > results later today. I'm going to step out for a bit. > > On 08/09/2016 05:43 PM, Ilia Mirkin wrote: >> ping? do we want this? should i drop it? >> >> On Wed, Jul 13, 2016 at 3:37 PM, Ilia Mirkin wrote: >>> Thanks for confirming, Corentin. >>> >>> Ian, do you have any opinions on this? Seems like a fairly innocuous >>> thing to be doing... >>> >>> On Fri, Jul 8, 2016 at 3:39 PM, Corentin Wallez wrote: Not sure how reviews work in Mesa, but this patch LGTM. I also tested that it fixes the relevant tests failures it is supposed to address. On Wed, Jul 6, 2016 at 7:40 PM, Ilia Mirkin wrote: > > The GL spec is very unclear on this point. Apparently this is discussed > without resolution in the closed Khronos bugtracker at > https://cvs.khronos.org/bugzilla/show_bug.cgi?id=7829 . The > recommendation is to allow dropping the [0] for looking up the bindings. > > The approach taken in this patch is to instead tack on [0]'s for each > arrayness level of the output's type, and doing the lookup again. That > way, for > > out vec4 foo[2][2][2] > > we will end up looking for bindings for foo, foo[0], foo[0][0], and > foo[0][0][0], in that order of preference. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96765 > Signed-off-by: Ilia Mirkin > --- > src/compiler/glsl/linker.cpp | 39 --- > 1 file changed, 28 insertions(+), 11 deletions(-) > > diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp > index d963f54..9d54c2f 100644 > --- a/src/compiler/glsl/linker.cpp > +++ b/src/compiler/glsl/linker.cpp > @@ -2566,6 +2566,7 @@ find_available_slots(unsigned used_mask, unsigned > needed_count) > /** > * Assign locations for either VS inputs or FS outputs > * > + * \param mem_ctx Temporary ralloc context used for linking > * \param prog Shader program whose variables need locations > assigned > * \param constants Driver specific constant values for the program. > * \param target_index Selector for the program target to receive > location > @@ -2577,7 +2578,8 @@ find_available_slots(unsigned used_mask, unsigned > needed_count) > * error is emitted to the shader link log and false is returned. > */ > bool > -assign_attribute_or_color_locations(gl_shader_program *prog, > +assign_attribute_or_color_locations(void *mem_ctx, > +gl_shader_program *prog, > struct gl_constants *constants, > unsigned target_index) > { > @@ -2680,16 +2682,31 @@ > assign_attribute_or_color_locations(gl_shader_program *prog, >} else if (target_index == MESA_SHADER_FRAGMENT) { > unsigned binding; > unsigned index; > + const char *name = var->name; > + const glsl_type *type = var->type; > + > + while (type) { > +/* Check if there's a binding for the variable name */ > +if (prog->FragDataBindings->get(binding, name)) { > + assert(binding >= FRAG_RESULT_DATA0); > + var->data.location = binding; > + var->data.is_unmatched_generic_inout = 0; > + > + if (prog->FragDataIndexBindings->get(index, name)) { > + var->data.index = index; > + } > + break; > +} > > -if (prog->FragDataBindings->get(binding, var->name)) { > - assert(binding >= FRAG_RESULT_DATA0); > - var->data.location = binding; > -var->data.is_unmatched_generic_inout = 0; > +/* If not, but it's an array type, look for name[0] */ > +if (type->is_array()) { > + name = ralloc_asprintf(mem_ctx, "%s[0]", name); > + type = type->fields.array; > + continue; > +} > > - if (prog->FragDataIndexBindings->get(index, var->name)) { > - var->data.index = index; > - } > -} > +break; > + } >} > >/* From GL4.5 core spec, section 15.2 (Shader Execution): > @@ -4816,12 +4833,12 @@ link_shaders(struct gl_context *ctx, struct > gl_shader_program *prog) >prev = i; > } > > - if (!assign_attribute_or_color_locations(prog, &ctx->Const, > + if (!assign_attribute_or_color_loca
[Mesa-dev] [PATCH] anv: Use macro to avoid code duplication for sample positions
Suggested-by: Jason Ekstrand Signed-off-by: Anuj Phogat --- src/intel/vulkan/genX_multisample.h | 95 +++ src/intel/vulkan/genX_pipeline_util.h | 35 ++--- src/intel/vulkan/genX_state.c | 68 +++-- 3 files changed, 106 insertions(+), 92 deletions(-) create mode 100644 src/intel/vulkan/genX_multisample.h diff --git a/src/intel/vulkan/genX_multisample.h b/src/intel/vulkan/genX_multisample.h new file mode 100644 index 000..0deb48f --- /dev/null +++ b/src/intel/vulkan/genX_multisample.h @@ -0,0 +1,95 @@ +/* + * Copyright © 2016 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * 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 NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS 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. + */ +#pragma once + +#define SAMPLE_POS_1X(prefix) \ +prefix##0XOffset = 0.5; \ +prefix##0YOffset = 0.5; + +#define SAMPLE_POS_2X(prefix) \ +prefix##0XOffset = 0.25; \ +prefix##0YOffset = 0.25; \ +prefix##1XOffset = 0.75; \ +prefix##1YOffset = 0.75; + +#define SAMPLE_POS_4X(prefix) \ +prefix##0XOffset = 0.375; \ +prefix##0YOffset = 0.125; \ +prefix##1XOffset = 0.875; \ +prefix##1YOffset = 0.375; \ +prefix##2XOffset = 0.125; \ +prefix##2YOffset = 0.625; \ +prefix##3XOffset = 0.625; \ +prefix##3YOffset = 0.875; + +#define SAMPLE_POS_8X(prefix) \ +prefix##0XOffset = 0.5625; \ +prefix##0YOffset = 0.3125; \ +prefix##1XOffset = 0.4375; \ +prefix##1YOffset = 0.6875; \ +prefix##2XOffset = 0.8125; \ +prefix##2YOffset = 0.5625; \ +prefix##3XOffset = 0.3125; \ +prefix##3YOffset = 0.1875; \ +prefix##4XOffset = 0.1875; \ +prefix##4YOffset = 0.8125; \ +prefix##5XOffset = 0.0625; \ +prefix##5YOffset = 0.4375; \ +prefix##6XOffset = 0.6875; \ +prefix##6YOffset = 0.9375; \ +prefix##7XOffset = 0.9375; \ +prefix##7YOffset = 0.0625; + +#define SAMPLE_POS_16X(prefix) \ +prefix##0XOffset = 0.5625; \ +prefix##0YOffset = 0.5625; \ +prefix##1XOffset = 0.4375; \ +prefix##1YOffset = 0.3125; \ +prefix##2XOffset = 0.3125; \ +prefix##2YOffset = 0.6250; \ +prefix##3XOffset = 0.7500; \ +prefix##3YOffset = 0.4375; \ +prefix##4XOffset = 0.1875; \ +prefix##4YOffset = 0.3750; \ +prefix##5XOffset = 0.6250; \ +prefix##5YOffset = 0.8125; \ +prefix##6XOffset = 0.8125; \ +prefix##6YOffset = 0.6875; \ +prefix##7XOffset = 0.6875; \ +prefix##7YOffset = 0.1875; \ +prefix##8XOffset = 0.3750; \ +prefix##8YOffset = 0.8750; \ +prefix##9XOffset = 0.5000; \ +prefix##9YOffset = 0.0625; \ +prefix##10XOffset = 0.2500; \ +prefix##10YOffset = 0.1250; \ +prefix##11XOffset = 0.1250; \ +prefix##11YOffset = 0.7500; \ +prefix##12XOffset = 0.; \ +prefix##12YOffset = 0.5000; \ +prefix##13XOffset = 0.9375; \ +prefix##13YOffset = 0.2500; \ +prefix##14XOffset = 0.8750; \ +prefix##14YOffset = 0.9375; \ +prefix##15XOffset = 0.0625; \ +prefix##15YOffset = 0.; diff --git a/src/intel/vulkan/genX_pipeline_util.h b/src/intel/vulkan/genX_pipeline_util.h index 64b89cd..cf2adb0 100644 --- a/src/intel/vulkan/genX_pipeline_util.h +++ b/src/intel/vulkan/genX_pipeline_util.h @@ -22,6 +22,7 @@ */ #include "vk_format_info.h" +#include "genX_multisample.h" static uint32_t vertex_element_comp_control(enum isl_format format, unsigned comp) @@ -488,42 +489,16 @@ emit_ms_state(struct anv_pipeline *pipeline, switch (samples) { case 1: - ms.Sample0XOffset = 0.5; - ms.Sample0YOffset = 0.5; + SAMPLE_POS_1X(ms.Sample); break; case 2: - ms.Sample0XOffset = 0.25; - ms.Sample0YOffset = 0.25; - ms.Sample1XOffset = 0.75; - ms.Sample1YOffset = 0.75; + SAMPLE_POS_2X(ms.Sample); break; case 4: - ms.Sample0XOffset = 0.375; - ms.Sample0YOffset = 0.125; - ms.Sample1XOffset = 0.875; - ms.Sample1YOffset = 0.375; - ms.Sa
[Mesa-dev] [PATCH] i965: Change 8x multisample positions
There are no standard sample positions defined in OpenGL and OpenGL ES specs. Implementations have the freedom to pick the positions which give plausible results. But the Vulkan 1.0 spec does define standard sample positions for different sample counts. Defined positions in Vulkan for all the sample counts except 8X match with the positions we set in i965. We have an upcoming plan to share the blorp code between OpenGL and Vulkan driver in near future. Keeping the 8X sample positions same on both the drivers will help us move in that direction. Here is an argument by Neil Roberts (from commit 20250e85) against any advantage of current 8X sample positions over the new ones: "The comment above for the 8x sample positions says that the hardware implements centroid interpolation by picking the centre-most sample that is inside the primitive. That implies that it might be worthwhile to pick a pattern that includes 0.5,0.5. However by experimentation this doesn't seem to actually be the case. With the sample positions in this patch, if I modify the piglit test below so that it instead reports the centroid position, it reports 0.492188,0.421875 which doesn't match any of the positions. If I modify the sample positions so that they include one at exactly 0.5,0.5 it doesn't help and it reports another position which is even further from the center for some reason. arb_gpu_shader5-interpolateAtSample-different Kenneth Graunke experimented with some other patterns that have a higher standard deviation but I think after some discussion it was decided that it would be better to pick the same pattern as the other graphics API in case there are games that rely on this pattern." Observed no regressions in jenkins testing. Signed-off-by: Anuj Phogat Cc: Jason Ekstrand --- src/mesa/drivers/dri/i965/brw_multisample_state.h | 33 --- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_multisample_state.h b/src/mesa/drivers/dri/i965/brw_multisample_state.h index 42a7fd3..78fb03a 100644 --- a/src/mesa/drivers/dri/i965/brw_multisample_state.h +++ b/src/mesa/drivers/dri/i965/brw_multisample_state.h @@ -46,20 +46,9 @@ static const uint32_t brw_multisample_positions_4x = 0xae2ae662; /** - * Sample positions are based on a solution to the "8 queens" puzzle. - * Rationale: in a solution to the 8 queens puzzle, no two queens share - * a row, column, or diagonal. This is a desirable property for samples - * in a multisampling pattern, because it ensures that the samples are - * relatively uniformly distributed through the pixel. - * - * There are several solutions to the 8 queens puzzle (see - * http://en.wikipedia.org/wiki/Eight_queens_puzzle). This solution was - * chosen because it has a queen close to the center; this should - * improve the accuracy of centroid interpolation, since the hardware - * implements centroid interpolation by choosing the centermost sample - * that overlaps with the primitive being drawn. + * Sample positions: * - * Note: from the Ivy Bridge PRM, Vol2 Part1 p304 (3DSTATE_MULTISAMPLE: + * From the Ivy Bridge PRM, Vol2 Part1 p304 (3DSTATE_MULTISAMPLE: * Programming Notes): * * "When programming the sample offsets (for NUMSAMPLES_4 or _8 and @@ -70,17 +59,17 @@ brw_multisample_positions_4x = 0xae2ae662; * * Sample positions: * 1 3 5 7 9 b d f - * 1 5 - * 3 2 - * 5 6 - * 7 4 - * 9 0 - * b 3 - * d 1 - * f 7 + * 1 7 + * 3 3 + * 5 0 + * 7 5 + * 9 2 + * b 1 + * d 4 + * f 6 */ static const uint32_t -brw_multisample_positions_8x[] = { 0xdbb39d79, 0x3ff55117 }; +brw_multisample_positions_8x[] = { 0x53d97b95, 0xf1bf173d }; /** * Sample positions: -- 2.5.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] mesa: Add _mesa_enum_to_string2()
On 08/11/2016 10:24 AM, Ian Romanick wrote: On 08/11/2016 10:11 AM, Chad Versace wrote: The new function is identical to _mesa_enum_to_string(), except that it returns false if the given number is an invalid enum. Bundling validation and lookup into a single function allows us to do both with a single lookup into the enum table. Cc: Haixia Shi Change-Id: I83d9a6e53223d1a971cf8dc22cb34b05b48d7889 --- src/mapi/glapi/gen/gl_enums.py | 20 src/mesa/main/enums.h | 2 ++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/mapi/glapi/gen/gl_enums.py b/src/mapi/glapi/gen/gl_enums.py index 4fc43ab..af21aaf 100644 --- a/src/mapi/glapi/gen/gl_enums.py +++ b/src/mapi/glapi/gen/gl_enums.py @@ -89,6 +89,17 @@ static char token_tmp[20]; */ const char *_mesa_enum_to_string( int nr ) { + const char *str; + _mesa_enum_to_string2(nr, &str); + return str; +} + +/** + * Identical to _mesa_enum_to_string(), except that this function returns false + * when the number is an invalid enum. + */ +bool _mesa_enum_to_string2(int nr, const char **str) +{ enum_elt *elt; Add an assert(str != NULL); and maybe document that in the function header. Done. The hunk now starts with this: /** * Identical to _mesa_enum_to_string(), except that this function returns false * when the number is an invalid enum. The out parameter \\a str must be non-null. */ bool _mesa_enum_to_string2(int nr, const char **str) { enum_elt *elt; assert(str != NULL); elt = ... ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: look for frag data bindings with [0] tacked onto the end for arrays
I'll rebase and feed it through Jenkins (I was given a branch of late). On Thu, Aug 11, 2016 at 1:38 PM, Ian Romanick wrote: > Or not. :( The patch no longer applies, and I wasn't able to trivially > make it apply. I suspect one of Timothy's recent patches is to blame. > > On 08/11/2016 10:06 AM, Ian Romanick wrote: >> This seems okay... let me run it through our CI first. I should have >> results later today. I'm going to step out for a bit. >> >> On 08/09/2016 05:43 PM, Ilia Mirkin wrote: >>> ping? do we want this? should i drop it? >>> >>> On Wed, Jul 13, 2016 at 3:37 PM, Ilia Mirkin wrote: Thanks for confirming, Corentin. Ian, do you have any opinions on this? Seems like a fairly innocuous thing to be doing... On Fri, Jul 8, 2016 at 3:39 PM, Corentin Wallez wrote: > Not sure how reviews work in Mesa, but this patch LGTM. I also tested that > it fixes the relevant tests failures it is supposed to address. > > On Wed, Jul 6, 2016 at 7:40 PM, Ilia Mirkin wrote: >> >> The GL spec is very unclear on this point. Apparently this is discussed >> without resolution in the closed Khronos bugtracker at >> https://cvs.khronos.org/bugzilla/show_bug.cgi?id=7829 . The >> recommendation is to allow dropping the [0] for looking up the bindings. >> >> The approach taken in this patch is to instead tack on [0]'s for each >> arrayness level of the output's type, and doing the lookup again. That >> way, for >> >> out vec4 foo[2][2][2] >> >> we will end up looking for bindings for foo, foo[0], foo[0][0], and >> foo[0][0][0], in that order of preference. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96765 >> Signed-off-by: Ilia Mirkin >> --- >> src/compiler/glsl/linker.cpp | 39 >> --- >> 1 file changed, 28 insertions(+), 11 deletions(-) >> >> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp >> index d963f54..9d54c2f 100644 >> --- a/src/compiler/glsl/linker.cpp >> +++ b/src/compiler/glsl/linker.cpp >> @@ -2566,6 +2566,7 @@ find_available_slots(unsigned used_mask, unsigned >> needed_count) >> /** >> * Assign locations for either VS inputs or FS outputs >> * >> + * \param mem_ctx Temporary ralloc context used for linking >> * \param prog Shader program whose variables need locations >> assigned >> * \param constants Driver specific constant values for the program. >> * \param target_index Selector for the program target to receive >> location >> @@ -2577,7 +2578,8 @@ find_available_slots(unsigned used_mask, unsigned >> needed_count) >> * error is emitted to the shader link log and false is returned. >> */ >> bool >> -assign_attribute_or_color_locations(gl_shader_program *prog, >> +assign_attribute_or_color_locations(void *mem_ctx, >> +gl_shader_program *prog, >> struct gl_constants *constants, >> unsigned target_index) >> { >> @@ -2680,16 +2682,31 @@ >> assign_attribute_or_color_locations(gl_shader_program *prog, >>} else if (target_index == MESA_SHADER_FRAGMENT) { >> unsigned binding; >> unsigned index; >> + const char *name = var->name; >> + const glsl_type *type = var->type; >> + >> + while (type) { >> +/* Check if there's a binding for the variable name */ >> +if (prog->FragDataBindings->get(binding, name)) { >> + assert(binding >= FRAG_RESULT_DATA0); >> + var->data.location = binding; >> + var->data.is_unmatched_generic_inout = 0; >> + >> + if (prog->FragDataIndexBindings->get(index, name)) { >> + var->data.index = index; >> + } >> + break; >> +} >> >> -if (prog->FragDataBindings->get(binding, var->name)) { >> - assert(binding >= FRAG_RESULT_DATA0); >> - var->data.location = binding; >> -var->data.is_unmatched_generic_inout = 0; >> +/* If not, but it's an array type, look for name[0] */ >> +if (type->is_array()) { >> + name = ralloc_asprintf(mem_ctx, "%s[0]", name); >> + type = type->fields.array; >> + continue; >> +} >> >> - if (prog->FragDataIndexBindings->get(index, var->name)) { >> - var->data.index = index; >> - } >> -} >> +break; >> + } >>} >> >>/* From GL4.5 core spec, section 15.2 (Shader Execution): >> @@
Re: [Mesa-dev] [PATCH] anv/device: Add limits for InterpolationOffset
On Fri, Jul 29, 2016 at 2:30 PM, Anuj Phogat wrote: > > > On Fri, Jul 29, 2016 at 12:32 PM, Anuj Phogat wrote: >> >> >> >> On Thu, Jul 28, 2016 at 6:31 PM, Jason Ekstrand >> wrote: >>> >>> On Jul 28, 2016 7:37 PM, "Anuj Phogat" wrote: >>> > >>> > Fixes the vulkan cts regression in test >>> > dEQP-VK.api.info.device.properties >>> > >>> > Cc: Mark Janes >>> > Cc: Jason Ekstrand >>> > Signed-off-by: Anuj Phogat >>> > --- >>> > src/intel/vulkan/anv_device.c | 6 +++--- >>> > 1 file changed, 3 insertions(+), 3 deletions(-) >>> > >>> > diff --git a/src/intel/vulkan/anv_device.c >>> > b/src/intel/vulkan/anv_device.c >>> > index c2c5153..9fd9683 100644 >>> > --- a/src/intel/vulkan/anv_device.c >>> > +++ b/src/intel/vulkan/anv_device.c >>> > @@ -524,9 +524,9 @@ void anv_GetPhysicalDeviceProperties( >>> >.maxTexelOffset = 7, >>> >.minTexelGatherOffset = -8, >>> >.maxTexelGatherOffset = 7, >>> > - .minInterpolationOffset = 0, /* FIXME */ >>> > - .maxInterpolationOffset = 0, /* FIXME */ >>> > - .subPixelInterpolationOffsetBits = 0, /* FIXME */ >>> > + .minInterpolationOffset = -0.5, >>> > + .maxInterpolationOffset = 0.5, > > Correction: maxInterpolationOffset = 0.4375 > From Vulkan 1.0 spec: > " > If subPixelInterpolationOffsetBits is 4, this provides increments of (1/24) > = 0.0625, and thus the range of supported interpolation offsets would be > [-0.5, 0.4375]. > " Jason, other than above correction, is there anything else you want me to address in V2 of this patch ? >>> >>> > + .subPixelInterpolationOffsetBits = 4, >>> >>> I looked and neither gen7_pipeline.c nor gen8_pipeline.c set the subpixel >>> precision so we get a default of enum value 0 which is 8 bits. We should >>> probably be explicitly setting it to something. I think 4 is probably fine >>> but we should be seeing it. >> >> I think you're talking about "Vertex Sub Pixel Precision Select" in >> 3DSTATE_SF. Yes, we are setting a default value of zero which is 8 bits in >> both OpenGL and Vulkan. This meets the minimum requirement of 4 bits for >> GL_SUBPIXEL_BITS. Yes, it'll be nicer to make it more obvious what we are >> setting here. >> >> The value we are defining in this patch is subPixelInterpolationOffsetBits >> (GL_FRAGMENT_INTERPOLATION_OFFSET_BITS in OpenGL). I found no option of >> setting it on Intel Hardware. I just replicated what we do in i965 and made >> the CTS test happy again. >> >> Looks unrelated but there is this another thing 'viewportSubPixelBits' >> which is initialized to 13 in Vulkan but 0 (required min. value) in i965. >> Again found no option of setting on Intel hardware.So, a similar case to >> subPixelInterpolationOffsetBits. >>> >>> As a side note, I think we could probably combine 3DSTATE_SF and >>> 3DSTATE_RASTER setup between gen7 and gen8 (and move it to >>> genX_pipeline_until.h). We'd probably have to do some #define trick like we >>> did for 3DSTATE_SBE and 3DSTATE_SBE_SWIZ because the two packets are >>> combined on gen7 and split on gen8. >> >> I'll look in to it. >> >>> >>> >.maxFramebufferWidth = (1 << 14), >>> >.maxFramebufferHeight = (1 << 14), >>> >.maxFramebufferLayers = (1 << 10), >>> > -- >>> > 2.5.5 >>> > >> >> > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Change 8x multisample positions
On Thu, Aug 11, 2016 at 10:41 AM, Anuj Phogat wrote: > There are no standard sample positions defined in OpenGL and OpenGL > ES specs. Implementations have the freedom to pick the positions > which give plausible results. But the Vulkan 1.0 spec does define > standard sample positions for different sample counts. Defined > positions in Vulkan for all the sample counts except 8X match with > the positions we set in i965. We have an upcoming plan to share the > blorp code between OpenGL and Vulkan driver in near future. Keeping > the 8X sample positions same on both the drivers will help us move > in that direction. > > Here is an argument by Neil Roberts (from commit 20250e85) against > any advantage of current 8X sample positions over the new ones: > > "The comment above for the 8x sample positions says that the hardware > implements centroid interpolation by picking the centre-most sample > that is inside the primitive. That implies that it might be worthwhile > to pick a pattern that includes 0.5,0.5. However by experimentation > this doesn't seem to actually be the case. With the sample positions > in this patch, if I modify the piglit test below so that it instead > reports the centroid position, it reports 0.492188,0.421875 which > doesn't match any of the positions. If I modify the sample positions > so that they include one at exactly 0.5,0.5 it doesn't help and it > reports another position which is even further from the center for > some reason. > > arb_gpu_shader5-interpolateAtSample-different > > Kenneth Graunke experimented with some other patterns that have a > higher standard deviation but I think after some discussion it was > decided that it would be better to pick the same pattern as the other > graphics API in case there are games that rely on this pattern." > > Observed no regressions in jenkins testing. > Did you try the scaled blit tests? I bet we have to rework 8x scaled blit for the new positions. Thanks for working on this! It's going to make the blorp transition way easier. > Signed-off-by: Anuj Phogat > Cc: Jason Ekstrand > --- > src/mesa/drivers/dri/i965/brw_multisample_state.h | 33 > --- > 1 file changed, 11 insertions(+), 22 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_multisample_state.h > b/src/mesa/drivers/dri/i965/brw_multisample_state.h > index 42a7fd3..78fb03a 100644 > --- a/src/mesa/drivers/dri/i965/brw_multisample_state.h > +++ b/src/mesa/drivers/dri/i965/brw_multisample_state.h > @@ -46,20 +46,9 @@ static const uint32_t > brw_multisample_positions_4x = 0xae2ae662; > > /** > - * Sample positions are based on a solution to the "8 queens" puzzle. > - * Rationale: in a solution to the 8 queens puzzle, no two queens share > - * a row, column, or diagonal. This is a desirable property for samples > - * in a multisampling pattern, because it ensures that the samples are > - * relatively uniformly distributed through the pixel. > - * > - * There are several solutions to the 8 queens puzzle (see > - * http://en.wikipedia.org/wiki/Eight_queens_puzzle). This solution was > - * chosen because it has a queen close to the center; this should > - * improve the accuracy of centroid interpolation, since the hardware > - * implements centroid interpolation by choosing the centermost sample > - * that overlaps with the primitive being drawn. > + * Sample positions: > Might be nice to reference the Vulkan spec here. > * > - * Note: from the Ivy Bridge PRM, Vol2 Part1 p304 (3DSTATE_MULTISAMPLE: > + * From the Ivy Bridge PRM, Vol2 Part1 p304 (3DSTATE_MULTISAMPLE: > * Programming Notes): > * > * "When programming the sample offsets (for NUMSAMPLES_4 or _8 and > @@ -70,17 +59,17 @@ brw_multisample_positions_4x = 0xae2ae662; > * > * Sample positions: > * 1 3 5 7 9 b d f > - * 1 5 > - * 3 2 > - * 5 6 > - * 7 4 > - * 9 0 > - * b 3 > - * d 1 > - * f 7 > + * 1 7 > + * 3 3 > + * 5 0 > + * 7 5 > + * 9 2 > + * b 1 > + * d 4 > + * f 6 > */ > static const uint32_t > -brw_multisample_positions_8x[] = { 0xdbb39d79, 0x3ff55117 }; > +brw_multisample_positions_8x[] = { 0x53d97b95, 0xf1bf173d }; > > /** > * Sample positions: > -- > 2.5.5 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] mesa: Fix glFramebufferTexture* error codes
On 08/11/2016 10:22 AM, Ilia Mirkin wrote: On Thu, Aug 11, 2016 at 1:11 PM, Chad Versace wrote: If check_textarget() determined that textarget was incorrect, it emitted GL_INVALID_OPERATION. This is the correct behavior when textarget is a valid GLenum but an invalid parameter to the current variant of glFramebufferTexture*(). However, when textarget is not a GLenum at all, then the GL spec requires that GL_INVALID_ENUM be emitted. Fixes test dEQP-GLES3.functional.negative_api.buffer.framebuffer_texture2d. Cc: Haixia Shi Change-Id: I86c492f228720ec8cf9939e741cfc99a5d9fa1bc --- src/mesa/main/fbobject.c | 32 +--- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index 2c01526..3360940 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -3029,20 +3029,30 @@ check_textarget(struct gl_context *ctx, int dims, GLenum target, err = true; } - if (err) { - _mesa_error(ctx, GL_INVALID_OPERATION, - "%s(invalid textarget %s)", - caller, _mesa_enum_to_string(textarget)); - return false; + if (!err) { + /* Make sure textarget is consistent with the texture's type */ + if (target == GL_TEXTURE_CUBE_MAP) { + err = !_mesa_is_cube_face(textarget); + } else { + err = (target != textarget); + } } - /* Make sure textarget is consistent with the texture's type */ - err = (target == GL_TEXTURE_CUBE_MAP) ? - !_mesa_is_cube_face(textarget): (target != textarget); - if (err) { - _mesa_error(ctx, GL_INVALID_OPERATION, - "%s(mismatched texture target)", caller); + const char *enum_str; + + if (_mesa_enum_to_string2(textarget, &enum_str)) { so ... if I feed it, say, GL_CLIP_PLANE0 then it's INVALID_OPERATION but if I feed it 0x then it's INVALID_ENUM? Probably should just list out the valid enums here. I wouldn't be surprised if such a list already existed elsewhere in the file. I didn't find such a list. I'll resubmit the patch with those enums handled by a switch at the top of the function. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] mesa: Add _mesa_enum_to_string2()
On Thu, Aug 11, 2016 at 10:42 AM, Chad Versace wrote: > On 08/11/2016 10:24 AM, Ian Romanick wrote: >> >> On 08/11/2016 10:11 AM, Chad Versace wrote: >>> >>> The new function is identical to _mesa_enum_to_string(), except that it >>> returns false if the given number is an invalid enum. Bundling >>> validation and lookup into a single function allows us to do both with >>> a single lookup into the enum table. >>> >>> Cc: Haixia Shi >>> Change-Id: I83d9a6e53223d1a971cf8dc22cb34b05b48d7889 >>> --- >>> src/mapi/glapi/gen/gl_enums.py | 20 >>> src/mesa/main/enums.h | 2 ++ >>> 2 files changed, 18 insertions(+), 4 deletions(-) >>> >>> diff --git a/src/mapi/glapi/gen/gl_enums.py >>> b/src/mapi/glapi/gen/gl_enums.py >>> index 4fc43ab..af21aaf 100644 >>> --- a/src/mapi/glapi/gen/gl_enums.py >>> +++ b/src/mapi/glapi/gen/gl_enums.py >>> @@ -89,6 +89,17 @@ static char token_tmp[20]; >>> */ >>> const char *_mesa_enum_to_string( int nr ) >>> { >>> + const char *str; >>> + _mesa_enum_to_string2(nr, &str); >>> + return str; >>> +} >>> + >>> +/** >>> + * Identical to _mesa_enum_to_string(), except that this function >>> returns false >>> + * when the number is an invalid enum. >>> + */ >>> +bool _mesa_enum_to_string2(int nr, const char **str) >>> +{ >>> enum_elt *elt; >>> >> >> Add an >> >>assert(str != NULL); >> >> and maybe document that in the function header. > > > Done. > > The hunk now starts with this: > > /** > * Identical to _mesa_enum_to_string(), except that this function returns > false > * when the number is an invalid enum. The out parameter \\a str must be > non-null. > */ > bool _mesa_enum_to_string2(int nr, const char **str) Might as well make this BSD-style as well, with return type on its own line. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/5] i965/surface_formats: Don't advertise 8 or 16-bit RGB formats
On Tue, Jul 26, 2016 at 10:04:22PM -0700, Jason Ekstrand wrote: > We have implicitly been not advertising these formats since we had them > turned off in the format capabilities table. We are about to update that > table and this prevents a change in behavior. The only change in behavior > created by this patch is that we no longer advertise support for > R16G16B16_FLOAT which means that it's now renderable which seems like a > bonus. Maybe someday we'll want to change things to start supporting I don't understand this sentence. Assuming "renderable" means that we can render to a texture with such a format, why does preventing the user from creating an R16G16B16_FLOAT texture mean that it's now renderable? - Nanley > 16-bit RGB formats natively but, at the moment, there's no need. > --- > src/mesa/drivers/dri/i965/brw_surface_formats.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c > b/src/mesa/drivers/dri/i965/brw_surface_formats.c > index 2543f4b..69d3bd4 100644 > --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c > +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c > @@ -311,6 +311,16 @@ brw_init_surface_formats(struct brw_context *brw) >if (texture == 0 && format != MESA_FORMAT_RGBA_FLOAT32) >continue; > > + /* Don't advertisel 8 and 16-bit RGB formats to core mesa. This > ensures > + * that they are renderable from an API perspective since core mesa > will > + * fall back to RGBA or RGBX (we can't render to non-power-of-two > + * formats). For 8-bit, formats, this also keeps us from hitting some > + * nasty corners in intel_miptree_map_blit if you ever try to map one. > + */ > + int format_size = _mesa_get_format_bytes(format); > + if (format_size == 3 || format_size == 6) > + continue; > + >if (isl_format_supports_sampling(devinfo, texture) && >(isl_format_supports_filtering(devinfo, texture) || is_integer)) >ctx->TextureFormatSupported[format] = true; > -- > 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
[Mesa-dev] [PATCH 2/2] mesa: Fix glFramebufferTexture* error codes (v2)
If check_textarget() determined that textarget was incorrect, it emitted GL_INVALID_OPERATION. This is the correct behavior when target and textarget are mismatched but textarget is a valid textarget enum. When textarget is not a valid textarget enum, the GL spec requires that GL_INVALID_ENUM be emitted. Fixes test dEQP-GLES3.functional.negative_api.buffer.framebuffer_texture2d. v2: - Continue emitting GL_INVALID_OPERATION when textarget is a valid textarget enum mismatched with target. [idr and imirkin] Cc: Ian Romanick Cc: Ilia Mirkin Cc: Haixia Shi Change-Id: I86c492f228720ec8cf9939e741cfc99a5d9fa1bc --- I'm now checking that textarget is a valid textarget enum with a switch at the top of the function, which emits error GL_INVALID_ENUM. The switch lists every textarget I know of, but I'm not confident that it's correct. Should some textargets not be in the switch? src/mesa/main/fbobject.c | 33 ++--- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index 2c01526..76adb29 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -2979,6 +2979,32 @@ check_textarget(struct gl_context *ctx, int dims, GLenum target, { bool err = false; + /* Check that textarget is a valid textarget enum. */ + switch (textarget) { + case GL_TEXTURE_1D: + case GL_TEXTURE_1D_ARRAY: + case GL_TEXTURE_2D: + case GL_TEXTURE_2D_ARRAY: + case GL_TEXTURE_2D_MULTISAMPLE: + case GL_TEXTURE_2D_MULTISAMPLE_ARRAY: + case GL_TEXTURE_CUBE_MAP: + case GL_TEXTURE_CUBE_MAP_ARRAY: + case GL_TEXTURE_CUBE_MAP_POSITIVE_X: + case GL_TEXTURE_CUBE_MAP_NEGATIVE_X: + case GL_TEXTURE_CUBE_MAP_POSITIVE_Y: + case GL_TEXTURE_CUBE_MAP_NEGATIVE_Y: + case GL_TEXTURE_CUBE_MAP_POSITIVE_Z: + case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z: + case GL_TEXTURE_RECTANGLE: + case GL_TEXTURE_3D: + break; + default: + _mesa_error(ctx, GL_INVALID_ENUM, "%s(invalid textarget %s)", + caller, _mesa_enum_to_string(textarget)); + return false; + } + + /* Check that target and textarget match. */ switch (dims) { case 1: switch (textarget) { @@ -3029,13 +3055,6 @@ check_textarget(struct gl_context *ctx, int dims, GLenum target, err = true; } - if (err) { - _mesa_error(ctx, GL_INVALID_OPERATION, - "%s(invalid textarget %s)", - caller, _mesa_enum_to_string(textarget)); - return false; - } - /* Make sure textarget is consistent with the texture's type */ err = (target == GL_TEXTURE_CUBE_MAP) ? !_mesa_is_cube_face(textarget): (target != textarget); -- 2.9.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/3] mesa: Fix a dEQP test (v2)
Fixes test dEQP-GLES3.functional.negative_api.buffer.framebuffer_texture2d. I tested this patch for regressions on Skylake with: % ./deqp-gles3 --deqp-case= 'dEQP-GLES3.functional.negative_api.*' % ./deqp-gles3 --deqp-case='dEQP-GLES3.functional.fbo.color.texcube.*' This patch series lives at git://git.kiwitree.net/~chadv/mesa branch review/fix-deqp-framebuffertexture-error-v02 Chad Versace (3): mesa: Document that _mesa_enum_to_string() returns non-null mesa: Add _mesa_enum_to_string2() mesa: Fix glFramebufferTexture* error codes src/mapi/glapi/gen/gl_enums.py | 24 src/mesa/main/enums.h | 2 ++ src/mesa/main/fbobject.c | 32 +--- 3 files changed, 43 insertions(+), 15 deletions(-) -- 2.9.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] mesa: Document that _mesa_enum_to_string() returns non-null
It always returns non-null, even if the number is an invalid enum. Cc: Haixia Shi Reviewed-by: Ian Romanick Change-Id: I26e8843c96130be972e66f48a49e362442e1bf97 --- src/mapi/glapi/gen/gl_enums.py | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/mapi/glapi/gen/gl_enums.py b/src/mapi/glapi/gen/gl_enums.py index 62cc1b3..2eb9e16 100644 --- a/src/mapi/glapi/gen/gl_enums.py +++ b/src/mapi/glapi/gen/gl_enums.py @@ -83,7 +83,12 @@ static int compar_nr( const int *a, enum_elt *b ) static char token_tmp[20]; -const char *_mesa_enum_to_string( int nr ) +/** + * This function always returns a string. If the number is a valid enum, it + * returns the enum name. Otherwise, it returns a numeric string. + */ +const char * +_mesa_enum_to_string(int nr) { enum_elt *elt; -- 2.9.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/5] isl/formats: Update the table with more samplable formats
On Tue, Jul 26, 2016 at 10:04:24PM -0700, Jason Ekstrand wrote: > There were a lot of formats where support was added on Haswell or later but > we never updated the format table. > --- > src/intel/isl/isl_format.c | 30 +++--- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/src/intel/isl/isl_format.c b/src/intel/isl/isl_format.c > index 366d32e..73688a7 100644 > --- a/src/intel/isl/isl_format.c > +++ b/src/intel/isl/isl_format.c > @@ -218,8 +218,8 @@ static const struct surface_format_info format_info[] = { > SF(50, 50, x, x, x, x, x, x, x,x, P8A8_UNORM_PALETTE1) > SF( x, x, x, x, x, x, x, x, x,x, A1B5G5R5_UNORM) > SF(90, 90, x, x, 90, x, x, x, x,x, A4B4G4R4_UNORM) > - SF( x, x, x, x, x, x, x, x, x,x, L8A8_UINT) > - SF( x, x, x, x, x, x, x, x, x,x, L8A8_SINT) > + SF(90, 90, x, x, x, x, x, x, x,x, L8A8_UINT) > + SF(90, 90, x, x, x, x, x, x, x,x, L8A8_SINT) > SF( Y, Y, x, 45, Y, Y, Y, x, x,x, R8_UNORM) > SF( Y, Y, x, x, Y, 60, Y, x, x,x, R8_SNORM) > SF( Y, x, x, x, Y, x, Y, x, x,x, R8_SINT) > @@ -237,10 +237,10 @@ static const struct surface_format_info format_info[] = > { > SF(45, 45, x, x, x, x, x, x, x,x, P4A4_UNORM_PALETTE1) > SF(45, 45, x, x, x, x, x, x, x,x, A4P4_UNORM_PALETTE1) > SF( x, x, x, x, x, x, x, x, x,x, Y8_UNORM) > - SF( x, x, x, x, x, x, x, x, x,x, L8_UINT) > - SF( x, x, x, x, x, x, x, x, x,x, L8_SINT) > - SF( x, x, x, x, x, x, x, x, x,x, I8_UINT) > - SF( x, x, x, x, x, x, x, x, x,x, I8_SINT) > + SF(90, 90, x, x, x, x, x, x, x,x, L8_UINT) > + SF(90, 90, x, x, x, x, x, x, x,x, L8_SINT) > + SF(90, 90, x, x, x, x, x, x, x,x, I8_UINT) > + SF(90, 90, x, x, x, x, x, x, x,x, I8_SINT) > SF(45, 45, x, x, x, x, x, x, x,x, DXT1_RGB_SRGB) > SF( Y, Y, x, x, x, x, x, x, x,x, R1_UNORM) > SF( Y, Y, x, Y, Y, x, x, x, 60,x, YCRCB_NORMAL) > @@ -261,8 +261,8 @@ static const struct surface_format_info format_info[] = { > SF( Y, Y, x, x, x, x, x, x, x,x, DXT1_RGB) > /* smpl filt shad CK RT AB VB SO color ccs_e */ > SF( Y, Y, x, x, x, x, x, x, x,x, FXT1) > - SF( x, x, x, x, x, x, Y, x, x,x, R8G8B8_UNORM) > - SF( x, x, x, x, x, x, Y, x, x,x, R8G8B8_SNORM) > + SF(75, 75, x, x, x, x, Y, x, x,x, R8G8B8_UNORM) > + SF(75, 75, x, x, x, x, Y, x, x,x, R8G8B8_SNORM) > SF( x, x, x, x, x, x, Y, x, x,x, R8G8B8_SSCALED) > SF( x, x, x, x, x, x, Y, x, x,x, R8G8B8_USCALED) > SF( x, x, x, x, x, x, Y, x, x,x, R64G64B64A64_FLOAT) > @@ -270,8 +270,8 @@ static const struct surface_format_info format_info[] = { > SF( Y, Y, x, x, x, x, x, x, x,x, BC4_SNORM) > SF( Y, Y, x, x, x, x, x, x, x,x, BC5_SNORM) > SF(50, 50, x, x, x, x, 60, x, x,x, R16G16B16_FLOAT) > - SF( x, x, x, x, x, x, Y, x, x,x, R16G16B16_UNORM) > - SF( x, x, x, x, x, x, Y, x, x,x, R16G16B16_SNORM) > + SF(75, 75, x, x, x, x, Y, x, x,x, R16G16B16_UNORM) > + SF(75, 75, x, x, x, x, Y, x, x,x, R16G16B16_SNORM) > SF( x, x, x, x, x, x, Y, x, x,x, R16G16B16_SSCALED) > SF( x, x, x, x, x, x, Y, x, x,x, R16G16B16_USCALED) > SF(70, 70, x, x, x, x, x, x, x,x, BC6H_SF16) > @@ -279,7 +279,7 @@ static const struct surface_format_info format_info[] = { > SF(70, 70, x, x, x, x, x, x, x,x, BC7_UNORM_SRGB) > SF(70, 70, x, x, x, x, x, x, x,x, BC6H_UF16) > SF( x, x, x, x, x, x, x, x, x,x, PLANAR_420_8) > - SF( x, x, x, x, x, x, x, x, x,x, R8G8B8_UNORM_SRGB) > + SF(75, 75, x, x, x, x, x, x, x,x, R8G8B8_UNORM_SRGB) I could not find support for this change in Vol7 of the HSW PRM. I did find such support for BDW however. - Nanley > SF(80, 80, x, x, x, x, x, x, x,x, ETC1_RGB8) > SF(80, 80, x, x, x, x, x, x, x,x, ETC2_RGB8) > SF(80, 80, x, x, x, x, x, x, x,x, EAC_R11) > @@ -287,8 +287,8 @@ static const struct surface_format_info format_info[] = { > SF(80, 80, x, x, x, x, x, x, x,x, EAC_SIGNED_R11) > SF(80, 80, x, x, x, x, x, x, x,x, EAC_SIGNED_RG11) > SF(80, 80, x, x, x, x, x, x, x,x, ETC2_SRGB8) > - SF( x, x, x, x, x, x, 75, x, x,x, R16G16B16_UINT) > - SF( x, x, x, x, x, x, 75, x, x,x, R16G16B16_SINT) > + SF(90, 90, x, x, x, x, 75, x, x,x, R16G16B16_UINT) > + SF(90, 90, x, x, x, x, 75, x, x,x, R16G16B16_SINT
Re: [Mesa-dev] [PATCH] i965: Change 8x multisample positions
On Thu, Aug 11, 2016 at 10:53 AM, Jason Ekstrand wrote: > > > On Thu, Aug 11, 2016 at 10:41 AM, Anuj Phogat wrote: >> >> There are no standard sample positions defined in OpenGL and OpenGL >> ES specs. Implementations have the freedom to pick the positions >> which give plausible results. But the Vulkan 1.0 spec does define >> standard sample positions for different sample counts. Defined >> positions in Vulkan for all the sample counts except 8X match with >> the positions we set in i965. We have an upcoming plan to share the >> blorp code between OpenGL and Vulkan driver in near future. Keeping >> the 8X sample positions same on both the drivers will help us move >> in that direction. >> >> Here is an argument by Neil Roberts (from commit 20250e85) against >> any advantage of current 8X sample positions over the new ones: >> >> "The comment above for the 8x sample positions says that the hardware >> implements centroid interpolation by picking the centre-most sample >> that is inside the primitive. That implies that it might be worthwhile >> to pick a pattern that includes 0.5,0.5. However by experimentation >> this doesn't seem to actually be the case. With the sample positions >> in this patch, if I modify the piglit test below so that it instead >> reports the centroid position, it reports 0.492188,0.421875 which >> doesn't match any of the positions. If I modify the sample positions >> so that they include one at exactly 0.5,0.5 it doesn't help and it >> reports another position which is even further from the center for >> some reason. >> >> arb_gpu_shader5-interpolateAtSample-different >> >> Kenneth Graunke experimented with some other patterns that have a >> higher standard deviation but I think after some discussion it was >> decided that it would be better to pick the same pattern as the other >> graphics API in case there are games that rely on this pattern." >> >> Observed no regressions in jenkins testing. > > > Did you try the scaled blit tests? I bet we have to rework 8x scaled blit > for the new positions. > They all passed because I missed changing sample mapping pattern in blorp and the piglit tests. So, the incorrect test image perfectly matched the incorrect reference image. I'll send out patches to fix these issues. > Thanks for working on this! It's going to make the blorp transition way > easier. > >> >> Signed-off-by: Anuj Phogat >> Cc: Jason Ekstrand >> --- >> src/mesa/drivers/dri/i965/brw_multisample_state.h | 33 >> --- >> 1 file changed, 11 insertions(+), 22 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_multisample_state.h >> b/src/mesa/drivers/dri/i965/brw_multisample_state.h >> index 42a7fd3..78fb03a 100644 >> --- a/src/mesa/drivers/dri/i965/brw_multisample_state.h >> +++ b/src/mesa/drivers/dri/i965/brw_multisample_state.h >> @@ -46,20 +46,9 @@ static const uint32_t >> brw_multisample_positions_4x = 0xae2ae662; >> >> /** >> - * Sample positions are based on a solution to the "8 queens" puzzle. >> - * Rationale: in a solution to the 8 queens puzzle, no two queens share >> - * a row, column, or diagonal. This is a desirable property for samples >> - * in a multisampling pattern, because it ensures that the samples are >> - * relatively uniformly distributed through the pixel. >> - * >> - * There are several solutions to the 8 queens puzzle (see >> - * http://en.wikipedia.org/wiki/Eight_queens_puzzle). This solution was >> - * chosen because it has a queen close to the center; this should >> - * improve the accuracy of centroid interpolation, since the hardware >> - * implements centroid interpolation by choosing the centermost sample >> - * that overlaps with the primitive being drawn. >> + * Sample positions: > > > Might be nice to reference the Vulkan spec here. I'll add the Vulkan reference here. > >> >> * >> - * Note: from the Ivy Bridge PRM, Vol2 Part1 p304 (3DSTATE_MULTISAMPLE: >> + * From the Ivy Bridge PRM, Vol2 Part1 p304 (3DSTATE_MULTISAMPLE: >> * Programming Notes): >> * >> * "When programming the sample offsets (for NUMSAMPLES_4 or _8 and >> @@ -70,17 +59,17 @@ brw_multisample_positions_4x = 0xae2ae662; >> * >> * Sample positions: >> * 1 3 5 7 9 b d f >> - * 1 5 >> - * 3 2 >> - * 5 6 >> - * 7 4 >> - * 9 0 >> - * b 3 >> - * d 1 >> - * f 7 >> + * 1 7 >> + * 3 3 >> + * 5 0 >> + * 7 5 >> + * 9 2 >> + * b 1 >> + * d 4 >> + * f 6 >> */ >> static const uint32_t >> -brw_multisample_positions_8x[] = { 0xdbb39d79, 0x3ff55117 }; >> +brw_multisample_positions_8x[] = { 0x53d97b95, 0xf1bf173d }; >> >> /** >> * Sample positions: >> -- >> 2.5.5 >> > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 97285] Darkness in Dota 2 after Patch "Make Gallium's BlitFramebuffer follow the GL 4.4 sRGB rules"
https://bugs.freedesktop.org/show_bug.cgi?id=97285 --- Comment #11 from Marek Olšák --- The patch looks good to me. -- 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 2/2] nir/algebraic: Optimize common array indexing sequence
2016-08-11 18:19 GMT+02:00 Ian Romanick : > ping > > On 07/19/2016 10:37 PM, Ian Romanick wrote: >> From: Ian Romanick >> >> Some shaders include code that looks like: >> >>uniform int i; >>uniform vec4 bones[...]; >> >>foo(bones[i * 3], bones[i * 3 + 1], bones[i * 3 + 2]); >> >> CSE would do some work on this: >> >>x = i * 3 >>foo(bones[x], bones[x + 1], bones[x + 2]); >> >> The compiler may then add '<< 4 + base' to the index calculations. >> This results in expressions like >> >>x = i * 3 >>foo(bones[x << 4], bones[(x + 1) << 4], bones[(x + 2) << 4]); >> This may just be me being dense, but why is the compiler adding the shift? It seems like this would be "un-optimizing" and so it shouldn't be added? This patch is also a good argument for adding a constant reassociation pass. At least the (a * #b) << #c--->a * (#b << #c) would be caught by that. I was confused whether the optimization was preserving signed-ness correctly but after a couple rounds with myself I believe it is correct. The change looks good and is: Reviewed-by: Thomas Helland (Oh, and yes, I'm back to hobby mesa development after finally securing a new job and a new apartment) >> Just rearranging the math to produce (i * 48) + 16 saves an >> instruction, and it allows CSE to do more work. >> >>x = i * 48; >>foo(bones[x], bones[x + 16], bones[x + 32]); >> >> So, ~6 instructions becomes ~3. >> >> Some individual shader-db results look pretty bad. However, I have a >> really, really hard time believing the change in estimated cycles in, >> for example, 3dmmes-taiji/51.shader_test after looking that change in >> the generated code. >> >> G45 >> total instructions in shared programs: 4020840 -> 4010070 (-0.27%) >> instructions in affected programs: 177460 -> 166690 (-6.07%) >> helped: 894 >> HURT: 0 >> >> total cycles in shared programs: 98829000 -> 98784990 (-0.04%) >> cycles in affected programs: 3936648 -> 3892638 (-1.12%) >> helped: 894 >> HURT: 0 >> >> Ironlake >> total instructions in shared programs: 6418887 -> 6408117 (-0.17%) >> instructions in affected programs: 177460 -> 166690 (-6.07%) >> helped: 894 >> HURT: 0 >> >> total cycles in shared programs: 143504542 -> 143460532 (-0.03%) >> cycles in affected programs: 3936648 -> 3892638 (-1.12%) >> helped: 894 >> HURT: 0 >> >> Sandy Bridge >> total instructions in shared programs: 8357887 -> 8339251 (-0.22%) >> instructions in affected programs: 432715 -> 414079 (-4.31%) >> helped: 2795 >> HURT: 0 >> >> total cycles in shared programs: 118284184 -> 118207412 (-0.06%) >> cycles in affected programs: 6114626 -> 6037854 (-1.26%) >> helped: 2478 >> HURT: 317 >> >> Ivy Bridge >> total instructions in shared programs: 7669390 -> 7653822 (-0.20%) >> instructions in affected programs: 388234 -> 372666 (-4.01%) >> helped: 2795 >> HURT: 0 >> >> total cycles in shared programs: 68381982 -> 68263684 (-0.17%) >> cycles in affected programs: 1972658 -> 1854360 (-6.00%) >> helped: 2458 >> HURT: 307 >> >> Haswell >> total instructions in shared programs: 7082636 -> 7067068 (-0.22%) >> instructions in affected programs: 388234 -> 372666 (-4.01%) >> helped: 2795 >> HURT: 0 >> >> total cycles in shared programs: 68282020 -> 68164158 (-0.17%) >> cycles in affected programs: 1891820 -> 1773958 (-6.23%) >> helped: 2459 >> HURT: 261 >> >> Broadwell >> total instructions in shared programs: 9002466 -> 8985875 (-0.18%) >> instructions in affected programs: 658784 -> 642193 (-2.52%) >> helped: 2795 >> HURT: 5 >> >> total cycles in shared programs: 78503092 -> 78450404 (-0.07%) >> cycles in affected programs: 2873304 -> 2820616 (-1.83%) >> helped: 2275 >> HURT: 415 >> >> Skylake >> total instructions in shared programs: 9156978 -> 9140387 (-0.18%) >> instructions in affected programs: 682625 -> 666034 (-2.43%) >> helped: 2795 >> HURT: 5 >> >> total cycles in shared programs: 75591392 -> 75550574 (-0.05%) >> cycles in affected programs: 3192120 -> 3151302 (-1.28%) >> helped: 2271 >> HURT: 425 >> >> Signed-off-by: Ian Romanick >> --- >> src/compiler/nir/nir_opt_algebraic.py | 11 +++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/src/compiler/nir/nir_opt_algebraic.py >> b/src/compiler/nir/nir_opt_algebraic.py >> index 0f0896b..37cb700 100644 >> --- a/src/compiler/nir/nir_opt_algebraic.py >> +++ b/src/compiler/nir/nir_opt_algebraic.py >> @@ -119,6 +119,17 @@ optimizations = [ >> (('~fadd@64', a, ('fmul', c , ('fadd', b, ('fneg', a, >> ('flrp', a, b, c), '!options->lower_flrp64'), >> (('ffma', a, b, c), ('fadd', ('fmul', a, b), c), 'options->lower_ffma'), >> (('~fadd', ('fmul', a, b), c), ('ffma', a, b, c), 'options->fuse_ffma'), >> + >> + # (a * #b + #c) << #d >> + # ((a * #b) << #d) + (#c << #d) >> + # (a * (#b << #d)) + (#c << #d) >> + (('ishl', ('iadd', ('imul', a, '#b'), '#c'), '#d'), >> +('iadd', ('imul', a, ('ishl', b, d)), ('ishl', c, d))), >> + >> + # (a * #b) << #c >> + # a * (#b << #c) >> + (('
Re: [Mesa-dev] [PATCH 3/5] isl/formats: Update the table with more samplable formats
On Tue, Jul 26, 2016 at 10:04:24PM -0700, Jason Ekstrand wrote: > There were a lot of formats where support was added on Haswell or later but > we never updated the format table. > --- > src/intel/isl/isl_format.c | 30 +++--- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/src/intel/isl/isl_format.c b/src/intel/isl/isl_format.c > index 366d32e..73688a7 100644 > --- a/src/intel/isl/isl_format.c > +++ b/src/intel/isl/isl_format.c > @@ -218,8 +218,8 @@ static const struct surface_format_info format_info[] = { > SF(50, 50, x, x, x, x, x, x, x,x, P8A8_UNORM_PALETTE1) > SF( x, x, x, x, x, x, x, x, x,x, A1B5G5R5_UNORM) > SF(90, 90, x, x, 90, x, x, x, x,x, A4B4G4R4_UNORM) > - SF( x, x, x, x, x, x, x, x, x,x, L8A8_UINT) > - SF( x, x, x, x, x, x, x, x, x,x, L8A8_SINT) > + SF(90, 90, x, x, x, x, x, x, x,x, L8A8_UINT) > + SF(90, 90, x, x, x, x, x, x, x,x, L8A8_SINT) How were you able to determine that these formats are filterable? In this table, it generally seems to be the case that integer formats are not filterable. - Nanley > SF( Y, Y, x, 45, Y, Y, Y, x, x,x, R8_UNORM) > SF( Y, Y, x, x, Y, 60, Y, x, x,x, R8_SNORM) > SF( Y, x, x, x, Y, x, Y, x, x,x, R8_SINT) > @@ -237,10 +237,10 @@ static const struct surface_format_info format_info[] = > { > SF(45, 45, x, x, x, x, x, x, x,x, P4A4_UNORM_PALETTE1) > SF(45, 45, x, x, x, x, x, x, x,x, A4P4_UNORM_PALETTE1) > SF( x, x, x, x, x, x, x, x, x,x, Y8_UNORM) > - SF( x, x, x, x, x, x, x, x, x,x, L8_UINT) > - SF( x, x, x, x, x, x, x, x, x,x, L8_SINT) > - SF( x, x, x, x, x, x, x, x, x,x, I8_UINT) > - SF( x, x, x, x, x, x, x, x, x,x, I8_SINT) > + SF(90, 90, x, x, x, x, x, x, x,x, L8_UINT) > + SF(90, 90, x, x, x, x, x, x, x,x, L8_SINT) > + SF(90, 90, x, x, x, x, x, x, x,x, I8_UINT) > + SF(90, 90, x, x, x, x, x, x, x,x, I8_SINT) > SF(45, 45, x, x, x, x, x, x, x,x, DXT1_RGB_SRGB) > SF( Y, Y, x, x, x, x, x, x, x,x, R1_UNORM) > SF( Y, Y, x, Y, Y, x, x, x, 60,x, YCRCB_NORMAL) > @@ -261,8 +261,8 @@ static const struct surface_format_info format_info[] = { > SF( Y, Y, x, x, x, x, x, x, x,x, DXT1_RGB) > /* smpl filt shad CK RT AB VB SO color ccs_e */ > SF( Y, Y, x, x, x, x, x, x, x,x, FXT1) > - SF( x, x, x, x, x, x, Y, x, x,x, R8G8B8_UNORM) > - SF( x, x, x, x, x, x, Y, x, x,x, R8G8B8_SNORM) > + SF(75, 75, x, x, x, x, Y, x, x,x, R8G8B8_UNORM) > + SF(75, 75, x, x, x, x, Y, x, x,x, R8G8B8_SNORM) > SF( x, x, x, x, x, x, Y, x, x,x, R8G8B8_SSCALED) > SF( x, x, x, x, x, x, Y, x, x,x, R8G8B8_USCALED) > SF( x, x, x, x, x, x, Y, x, x,x, R64G64B64A64_FLOAT) > @@ -270,8 +270,8 @@ static const struct surface_format_info format_info[] = { > SF( Y, Y, x, x, x, x, x, x, x,x, BC4_SNORM) > SF( Y, Y, x, x, x, x, x, x, x,x, BC5_SNORM) > SF(50, 50, x, x, x, x, 60, x, x,x, R16G16B16_FLOAT) > - SF( x, x, x, x, x, x, Y, x, x,x, R16G16B16_UNORM) > - SF( x, x, x, x, x, x, Y, x, x,x, R16G16B16_SNORM) > + SF(75, 75, x, x, x, x, Y, x, x,x, R16G16B16_UNORM) > + SF(75, 75, x, x, x, x, Y, x, x,x, R16G16B16_SNORM) > SF( x, x, x, x, x, x, Y, x, x,x, R16G16B16_SSCALED) > SF( x, x, x, x, x, x, Y, x, x,x, R16G16B16_USCALED) > SF(70, 70, x, x, x, x, x, x, x,x, BC6H_SF16) > @@ -279,7 +279,7 @@ static const struct surface_format_info format_info[] = { > SF(70, 70, x, x, x, x, x, x, x,x, BC7_UNORM_SRGB) > SF(70, 70, x, x, x, x, x, x, x,x, BC6H_UF16) > SF( x, x, x, x, x, x, x, x, x,x, PLANAR_420_8) > - SF( x, x, x, x, x, x, x, x, x,x, R8G8B8_UNORM_SRGB) > + SF(75, 75, x, x, x, x, x, x, x,x, R8G8B8_UNORM_SRGB) > SF(80, 80, x, x, x, x, x, x, x,x, ETC1_RGB8) > SF(80, 80, x, x, x, x, x, x, x,x, ETC2_RGB8) > SF(80, 80, x, x, x, x, x, x, x,x, EAC_R11) > @@ -287,8 +287,8 @@ static const struct surface_format_info format_info[] = { > SF(80, 80, x, x, x, x, x, x, x,x, EAC_SIGNED_R11) > SF(80, 80, x, x, x, x, x, x, x,x, EAC_SIGNED_RG11) > SF(80, 80, x, x, x, x, x, x, x,x, ETC2_SRGB8) > - SF( x, x, x, x, x, x, 75, x, x,x, R16G16B16_UINT) > - SF( x, x, x, x, x, x, 75, x, x,x, R16G16B16_SINT) > + SF(90, 90, x, x, x, x, 75, x, x,x, R16G16B16_UINT) > + SF(90, 90,
[Mesa-dev] [PATCH V2 2/2] i965: Change 8X MSAA sample mapping
This is required following the change in 8X sample positions. Fixes the recently modified multisample-scaled-blit piglit tests. Signed-off-by: Anuj Phogat --- src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 10 +- src/mesa/drivers/dri/i965/gen6_multisample_state.c | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp index b903de1..f7f685b 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp @@ -1094,13 +1094,13 @@ blorp_nir_manual_blend_bilinear(nir_builder *b, nir_ssa_def *pos, * * In case of 8x MSAA the two layouts don't match. * sample index layout : -sample number layout : - - *| 0 | 1 || 5 | 2 | + *| 0 | 1 || 3 | 7 | *-- - *| 2 | 3 || 4 | 6 | + *| 2 | 3 || 5 | 0 | *-- - *| 4 | 5 || 0 | 3 | + *| 4 | 5 || 1 | 2 | *-- - *| 6 | 7 || 7 | 1 | + *| 6 | 7 || 4 | 6 | *-- * * Fortunately, this can be done fairly easily as: @@ -1128,7 +1128,7 @@ blorp_nir_manual_blend_bilinear(nir_builder *b, nir_ssa_def *pos, sample = nir_f2i(b, sample); if (tex_samples == 8) { - sample = nir_iand(b, nir_ishr(b, nir_imm_int(b, 0x17306425), + sample = nir_iand(b, nir_ishr(b, nir_imm_int(b, 0x64210573), nir_ishl(b, sample, nir_imm_int(b, 2))), nir_imm_int(b, 0xf)); } else if (tex_samples == 16) { diff --git a/src/mesa/drivers/dri/i965/gen6_multisample_state.c b/src/mesa/drivers/dri/i965/gen6_multisample_state.c index a47e323..a59ffec 100644 --- a/src/mesa/drivers/dri/i965/gen6_multisample_state.c +++ b/src/mesa/drivers/dri/i965/gen6_multisample_state.c @@ -109,7 +109,7 @@ gen6_set_sample_maps(struct gl_context *ctx) { uint8_t map_2x[2] = {0, 1}; uint8_t map_4x[4] = {0, 1, 2, 3}; - uint8_t map_8x[8] = {5, 2, 4, 6, 0, 3, 7, 1}; + uint8_t map_8x[8] = {3, 7, 5, 0, 1, 2, 4, 6}; uint8_t map_16x[16] = { 15, 10, 9, 7, 4, 1, 3, 13, 12, 2, 0, 6, 11, 8, 5, 14 }; -- 2.5.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH V2 1/2] i965: Change 8x multisample positions
There are no standard sample positions defined in OpenGL and OpenGL ES specs. Implementations have the freedom to pick the positions which give plausible results. But the Vulkan 1.0 spec does define standard sample positions for different sample counts. Defined positions in Vulkan for all the sample counts except 8X match with the positions we set in i965. We have an upcoming plan to share the blorp code between OpenGL and Vulkan driver in near future. Keeping the 8X sample positions same on both the drivers will help us move in that direction. Here is an argument by Neil Roberts (from commit 20250e85) against any advantage of current 8X sample positions over the new ones: "The comment above for the 8x sample positions says that the hardware implements centroid interpolation by picking the centre-most sample that is inside the primitive. That implies that it might be worthwhile to pick a pattern that includes 0.5,0.5. However by experimentation this doesn't seem to actually be the case. With the sample positions in this patch, if I modify the piglit test below so that it instead reports the centroid position, it reports 0.492188,0.421875 which doesn't match any of the positions. If I modify the sample positions so that they include one at exactly 0.5,0.5 it doesn't help and it reports another position which is even further from the center for some reason. arb_gpu_shader5-interpolateAtSample-different Kenneth Graunke experimented with some other patterns that have a higher standard deviation but I think after some discussion it was decided that it would be better to pick the same pattern as the other graphics API in case there are games that rely on this pattern." Observed no regressions in jenkins testing. Signed-off-by: Anuj Phogat Cc: Jason Ekstrand --- src/mesa/drivers/dri/i965/brw_multisample_state.h | 43 +++ 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_multisample_state.h b/src/mesa/drivers/dri/i965/brw_multisample_state.h index 42a7fd3..db59af2 100644 --- a/src/mesa/drivers/dri/i965/brw_multisample_state.h +++ b/src/mesa/drivers/dri/i965/brw_multisample_state.h @@ -24,6 +24,15 @@ #include /** + * Note: There are no standard multisample positions defined in OpenGL + * specifications. Implementations have the freedom to pick the positions + * which give plausible results. But the Vulkan specification does define + * standard sample positions. So, we decided to pick the same pattern in + * OpenGL as in Vulkan to keep it uniform across drivers and also to avoid + * breaking applications which rely on this standard pattern. + */ + +/** * 1x MSAA has a single sample at the center: (0.5, 0.5) -> (0x8, 0x8). * * 2x MSAA sample positions are (0.25, 0.25) and (0.75, 0.75): @@ -46,22 +55,10 @@ static const uint32_t brw_multisample_positions_4x = 0xae2ae662; /** - * Sample positions are based on a solution to the "8 queens" puzzle. - * Rationale: in a solution to the 8 queens puzzle, no two queens share - * a row, column, or diagonal. This is a desirable property for samples - * in a multisampling pattern, because it ensures that the samples are - * relatively uniformly distributed through the pixel. - * - * There are several solutions to the 8 queens puzzle (see - * http://en.wikipedia.org/wiki/Eight_queens_puzzle). This solution was - * chosen because it has a queen close to the center; this should - * improve the accuracy of centroid interpolation, since the hardware - * implements centroid interpolation by choosing the centermost sample - * that overlaps with the primitive being drawn. + * Sample positions: * - * Note: from the Ivy Bridge PRM, Vol2 Part1 p304 (3DSTATE_MULTISAMPLE: + * From the Ivy Bridge PRM, Vol2 Part1 p304 (3DSTATE_MULTISAMPLE: * Programming Notes): - * * "When programming the sample offsets (for NUMSAMPLES_4 or _8 and * MSRASTMODE_xxx_PATTERN), the order of the samples 0 to 3 (or 7 * for 8X) must have monotonically increasing distance from the @@ -70,17 +67,17 @@ brw_multisample_positions_4x = 0xae2ae662; * * Sample positions: * 1 3 5 7 9 b d f - * 1 5 - * 3 2 - * 5 6 - * 7 4 - * 9 0 - * b 3 - * d 1 - * f 7 + * 1 7 + * 3 3 + * 5 0 + * 7 5 + * 9 2 + * b 1 + * d 4 + * f 6 */ static const uint32_t -brw_multisample_positions_8x[] = { 0xdbb39d79, 0x3ff55117 }; +brw_multisample_positions_8x[] = { 0x53d97b95, 0xf1bf173d }; /** * Sample positions: -- 2.5.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 97291] Incorrect packing of struct
https://bugs.freedesktop.org/show_bug.cgi?id=97291 --- Comment #4 from Matias N. Goldberg --- OK I'm fully re-powered and thinking straight now: I've built Mesa from scratch. Tried commits git-edfc17a (current head of branch 12.0) and commit 17f1c49b9ad05af4f6482f6fa950e5dcc1a779d1 (current head of master branch) In both, I don't have compute shaders. Googling around it appears for Southern Island radeon it could be buggy so it was turned off. Not 100% sure but I think that's what's happening. Anyway, not a big deal on that. WIP. Move on. Like I said our Forward3D demo was glitching and I didn't know why. I've investigated the problem and located it: * glGetIntegerv w/ GL_UNIFORM_BUFFER_OFFSET_ALIGNMENT is returning 4, should return 256 for my HW. * glGetIntegerv w/ GL_TEXTURE_BUFFER_OFFSET_ALIGNMENT is returning 4, should return 256 for my HW. This is clearly not a shader issue which is why I reported it in this ticket: https://bugs.freedesktop.org/show_bug.cgi?id=97305 Thanks for assistance, and I'm sorry for having bothered you earlier. Cheers -- You are receiving this mail because: 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 0/5] isl: Update the format table and add asserts
On Tue, Jul 26, 2016 at 10:04:21PM -0700, Jason Ekstrand wrote: > The real objective of this series is patch 5 which prevents us from > accidentally creating a surface state with a format unsupported by the > hardware. This turns some of the new Vulkan CTS tests from a hang into an > informative crash. In order to get there, however, we needed to update the > format table in isl with some of the new formats added on Haswell and later > generations. In order to do that, we had to fix up the dri driver, and own > the rabbit hole we go! > Contrary to your introduction, I cannot see how patches 4 and 5 are dependent on patches 1-3. Could you please explain this further? - Nanley > At the end of the series, the hangs in the latest CTS are gone (they came > from trying to clear an unsupported image format). > > Jason Ekstrand (5): > i965/surface_formats: Don't advertise 8 or 16-bit RGB formats > isl/formats: Report ETC as being samplable on Bay Trail > isl/formats: Update the table with more samplable formats > anv/image: Don't create invalid render target surfaces > isl/state: Add some asserts about format capabilities > > src/intel/isl/isl_format.c | 48 > + > src/intel/isl/isl_surface_state.c | 5 +++ > src/intel/vulkan/anv_image.c| 13 ++- > src/mesa/drivers/dri/i965/brw_surface_formats.c | 10 ++ > 4 files changed, 60 insertions(+), 16 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
[Mesa-dev] [PATCH 1/4] gallium/radeon: set SHADER_RW_BUFFER priority for streamout buffers
From: Marek Olšák --- src/gallium/drivers/radeon/r600_streamout.c | 4 ++-- src/gallium/drivers/radeonsi/si_descriptors.c | 6 -- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/gallium/drivers/radeon/r600_streamout.c b/src/gallium/drivers/radeon/r600_streamout.c index 705eb13..b5296aa 100644 --- a/src/gallium/drivers/radeon/r600_streamout.c +++ b/src/gallium/drivers/radeon/r600_streamout.c @@ -211,31 +211,31 @@ static void r600_emit_streamout_begin(struct r600_common_context *rctx, struct r update_flags |= SURFACE_BASE_UPDATE_STRMOUT(i); radeon_set_context_reg_seq(cs, R_028AD0_VGT_STRMOUT_BUFFER_SIZE_0 + 16*i, 3); radeon_emit(cs, (t[i]->b.buffer_offset + t[i]->b.buffer_size) >> 2);/* BUFFER_SIZE (in DW) */ radeon_emit(cs, stride_in_dw[i]); /* VTX_STRIDE (in DW) */ radeon_emit(cs, va >> 8); /* BUFFER_BASE */ r600_emit_reloc(rctx, &rctx->gfx, r600_resource(t[i]->b.buffer), - RADEON_USAGE_WRITE, RADEON_PRIO_RINGS_STREAMOUT); + RADEON_USAGE_WRITE, RADEON_PRIO_SHADER_RW_BUFFER); /* R7xx requires this packet after updating BUFFER_BASE. * Without this, R7xx locks up. */ if (rctx->family >= CHIP_RS780 && rctx->family <= CHIP_RV740) { radeon_emit(cs, PKT3(PKT3_STRMOUT_BASE_UPDATE, 1, 0)); radeon_emit(cs, i); radeon_emit(cs, va >> 8); r600_emit_reloc(rctx, &rctx->gfx, r600_resource(t[i]->b.buffer), - RADEON_USAGE_WRITE, RADEON_PRIO_RINGS_STREAMOUT); + RADEON_USAGE_WRITE, RADEON_PRIO_SHADER_RW_BUFFER); } } if (rctx->streamout.append_bitmask & (1 << i) && t[i]->buf_filled_size_valid) { uint64_t va = t[i]->buf_filled_size->gpu_address + t[i]->buf_filled_size_offset; /* Append. */ radeon_emit(cs, PKT3(PKT3_STRMOUT_BUFFER_UPDATE, 4, 0)); radeon_emit(cs, STRMOUT_SELECT_BUFFER(i) | diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c index 1d04a9c..fcc8a32 100644 --- a/src/gallium/drivers/radeonsi/si_descriptors.c +++ b/src/gallium/drivers/radeonsi/si_descriptors.c @@ -1304,21 +1304,22 @@ static void si_set_streamout_targets(struct pipe_context *ctx, S_008F0C_DST_SEL_Z(V_008F0C_SQ_SEL_Z) | S_008F0C_DST_SEL_W(V_008F0C_SQ_SEL_W) | S_008F0C_DATA_FORMAT(V_008F0C_BUF_DATA_FORMAT_32); /* Set the resource. */ pipe_resource_reference(&buffers->buffers[bufidx], buffer); radeon_add_to_buffer_list_check_mem(&sctx->b, &sctx->b.gfx, (struct r600_resource*)buffer, buffers->shader_usage, - buffers->priority, true); + RADEON_PRIO_SHADER_RW_BUFFER, + true); buffers->enabled_mask |= 1u << bufidx; } else { /* Clear the descriptor and unset the resource. */ memset(descs->list + bufidx*4, 0, sizeof(uint32_t) * 4); pipe_resource_reference(&buffers->buffers[bufidx], NULL); buffers->enabled_mask &= ~(1u << bufidx); } descs->dirty_mask |= 1u << bufidx; @@ -1467,21 +1468,22 @@ static void si_invalidate_buffer(struct pipe_context *ctx, struct pipe_resource if (buffers->buffers[i] != buf) continue; si_desc_reset_buffer_offset(ctx, descs->list + i*4, old_va, buf); descs->dirty_mask |= 1u << i; sctx->descriptors_dirty |= 1u << SI_DESCS_RW_BUFFERS; radeon_add_to_buffer_list_check_mem(&sctx->b, &sctx->b.gfx, rbuffer, buffers->shader_usage, - buffers->priority, true);
[Mesa-dev] [PATCH 4/4] gallium/radeon: assign the highest priority to scratch; make rings second
From: Marek Olšák just FYI, the kernel receives priority/4 --- src/gallium/drivers/radeon/radeon_winsys.h | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/radeon/radeon_winsys.h b/src/gallium/drivers/radeon/radeon_winsys.h index 8a98ebf..c65b9a4 100644 --- a/src/gallium/drivers/radeon/radeon_winsys.h +++ b/src/gallium/drivers/radeon/radeon_winsys.h @@ -190,40 +190,42 @@ enum radeon_bo_priority { RADEON_PRIO_INTERNAL_SHADER, /* fetch shader, etc. */ RADEON_PRIO_CONST_BUFFER = 16, RADEON_PRIO_DESCRIPTORS, RADEON_PRIO_BORDER_COLORS, RADEON_PRIO_SAMPLER_BUFFER = 20, RADEON_PRIO_VERTEX_BUFFER, RADEON_PRIO_SHADER_RW_BUFFER = 24, -RADEON_PRIO_SCRATCH_BUFFER, RADEON_PRIO_COMPUTE_GLOBAL, RADEON_PRIO_SAMPLER_TEXTURE = 28, RADEON_PRIO_SHADER_RW_IMAGE, RADEON_PRIO_SAMPLER_TEXTURE_MSAA = 32, RADEON_PRIO_COLOR_BUFFER = 36, RADEON_PRIO_DEPTH_BUFFER = 40, RADEON_PRIO_COLOR_BUFFER_MSAA = 44, RADEON_PRIO_DEPTH_BUFFER_MSAA = 48, RADEON_PRIO_CMASK = 52, RADEON_PRIO_DCC, RADEON_PRIO_HTILE, -RADEON_PRIO_SHADER_RINGS, + +RADEON_PRIO_SHADER_RINGS = 56, + +RADEON_PRIO_SCRATCH_BUFFER = 60, /* 63 is the maximum value */ }; struct winsys_handle; struct radeon_winsys_ctx; struct radeon_winsys_cs_chunk { unsigned cdw; /* Number of used dwords. */ unsigned max_dw; /* Maximum number of dwords. */ uint32_t *buf; /* The base pointer of the chunk. */ -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/4] gallium/radeon: mark shader rings as highest-priority buffers
From: Marek Olšák and rename the enum --- src/gallium/drivers/r600/evergreen_state.c| 4 ++-- src/gallium/drivers/r600/r600_state.c | 4 ++-- src/gallium/drivers/radeon/radeon_winsys.h| 2 +- src/gallium/drivers/radeonsi/si_debug.c | 2 +- src/gallium/drivers/radeonsi/si_descriptors.c | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c index 463dc15..7611520 100644 --- a/src/gallium/drivers/r600/evergreen_state.c +++ b/src/gallium/drivers/r600/evergreen_state.c @@ -2273,31 +2273,31 @@ static void evergreen_emit_gs_rings(struct r600_context *rctx, struct r600_atom radeon_emit(cs, PKT3(PKT3_EVENT_WRITE, 0, 0)); radeon_emit(cs, EVENT_TYPE(EVENT_TYPE_VGT_FLUSH)); if (state->enable) { rbuffer =(struct r600_resource*)state->esgs_ring.buffer; radeon_set_config_reg(cs, R_008C40_SQ_ESGS_RING_BASE, rbuffer->gpu_address >> 8); radeon_emit(cs, PKT3(PKT3_NOP, 0, 0)); radeon_emit(cs, radeon_add_to_buffer_list(&rctx->b, &rctx->b.gfx, rbuffer, RADEON_USAGE_READWRITE, - RADEON_PRIO_RINGS_STREAMOUT)); + RADEON_PRIO_SHADER_RINGS)); radeon_set_config_reg(cs, R_008C44_SQ_ESGS_RING_SIZE, state->esgs_ring.buffer_size >> 8); rbuffer =(struct r600_resource*)state->gsvs_ring.buffer; radeon_set_config_reg(cs, R_008C48_SQ_GSVS_RING_BASE, rbuffer->gpu_address >> 8); radeon_emit(cs, PKT3(PKT3_NOP, 0, 0)); radeon_emit(cs, radeon_add_to_buffer_list(&rctx->b, &rctx->b.gfx, rbuffer, RADEON_USAGE_READWRITE, - RADEON_PRIO_RINGS_STREAMOUT)); + RADEON_PRIO_SHADER_RINGS)); radeon_set_config_reg(cs, R_008C4C_SQ_GSVS_RING_SIZE, state->gsvs_ring.buffer_size >> 8); } else { radeon_set_config_reg(cs, R_008C44_SQ_ESGS_RING_SIZE, 0); radeon_set_config_reg(cs, R_008C4C_SQ_GSVS_RING_SIZE, 0); } radeon_set_config_reg(cs, R_008040_WAIT_UNTIL, S_008040_WAIT_3D_IDLE(1)); radeon_emit(cs, PKT3(PKT3_EVENT_WRITE, 0, 0)); radeon_emit(cs, EVENT_TYPE(EVENT_TYPE_VGT_FLUSH)); diff --git a/src/gallium/drivers/r600/r600_state.c b/src/gallium/drivers/r600/r600_state.c index 5b47089..046573f 100644 --- a/src/gallium/drivers/r600/r600_state.c +++ b/src/gallium/drivers/r600/r600_state.c @@ -1956,30 +1956,30 @@ static void r600_emit_gs_rings(struct r600_context *rctx, struct r600_atom *a) radeon_set_config_reg(cs, R_008040_WAIT_UNTIL, S_008040_WAIT_3D_IDLE(1)); radeon_emit(cs, PKT3(PKT3_EVENT_WRITE, 0, 0)); radeon_emit(cs, EVENT_TYPE(EVENT_TYPE_VGT_FLUSH)); if (state->enable) { rbuffer =(struct r600_resource*)state->esgs_ring.buffer; radeon_set_config_reg(cs, R_008C40_SQ_ESGS_RING_BASE, 0); radeon_emit(cs, PKT3(PKT3_NOP, 0, 0)); radeon_emit(cs, radeon_add_to_buffer_list(&rctx->b, &rctx->b.gfx, rbuffer, RADEON_USAGE_READWRITE, - RADEON_PRIO_RINGS_STREAMOUT)); + RADEON_PRIO_SHADER_RINGS)); radeon_set_config_reg(cs, R_008C44_SQ_ESGS_RING_SIZE, state->esgs_ring.buffer_size >> 8); rbuffer =(struct r600_resource*)state->gsvs_ring.buffer; radeon_set_config_reg(cs, R_008C48_SQ_GSVS_RING_BASE, 0); radeon_emit(cs, PKT3(PKT3_NOP, 0, 0)); radeon_emit(cs, radeon_add_to_buffer_list(&rctx->b, &rctx->b.gfx, rbuffer, RADEON_USAGE_READWRITE, - RADEON_PRIO_RINGS_STREAMOUT)); + RADEON_PRIO_SHADER_RINGS)); radeon_set_config_reg(cs, R_008C4C_SQ_GSVS_RING_SIZE, state->gsvs_ring.buffer_size >> 8); } else { radeon_set_config_reg(cs, R_008C44_SQ_ESGS_RING_SIZE, 0); radeon_set_config_reg(cs, R_008C4C_SQ_GSVS_RING_SIZE, 0); } radeon_set_config_reg(cs, R_008040_WAIT_UNTIL, S_008040_WAIT_3D_IDLE(1)); radeon_emit(cs, PKT3(PKT3_EVENT_WRITE, 0, 0)); radeon_emit(cs, EVENT_TYPE(EVENT_TYPE_VGT_FLUSH)); diff --git a/src/gallium/dri
[Mesa-dev] [PATCH 3/4] gallium/winsys: re-number winsys priority flags
From: Marek Olšák free 60..63, move CP_DMA up --- src/gallium/drivers/radeon/radeon_winsys.h | 29 + 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/gallium/drivers/radeon/radeon_winsys.h b/src/gallium/drivers/radeon/radeon_winsys.h index e4d669f..8a98ebf 100644 --- a/src/gallium/drivers/radeon/radeon_winsys.h +++ b/src/gallium/drivers/radeon/radeon_winsys.h @@ -173,57 +173,54 @@ enum radeon_bo_priority { RADEON_PRIO_FENCE = 0, RADEON_PRIO_TRACE, RADEON_PRIO_SO_FILLED_SIZE, RADEON_PRIO_QUERY, RADEON_PRIO_IB1 = 4, /* main IB submitted to the kernel */ RADEON_PRIO_IB2, /* IB executed with INDIRECT_BUFFER */ RADEON_PRIO_DRAW_INDIRECT, RADEON_PRIO_INDEX_BUFFER, -RADEON_PRIO_CP_DMA = 8, - -RADEON_PRIO_VCE = 12, +RADEON_PRIO_VCE = 8, RADEON_PRIO_UVD, RADEON_PRIO_SDMA_BUFFER, RADEON_PRIO_SDMA_TEXTURE, -RADEON_PRIO_USER_SHADER = 16, +RADEON_PRIO_CP_DMA = 12, +RADEON_PRIO_USER_SHADER, RADEON_PRIO_INTERNAL_SHADER, /* fetch shader, etc. */ -/* gap: 20 */ - -RADEON_PRIO_CONST_BUFFER = 24, +RADEON_PRIO_CONST_BUFFER = 16, RADEON_PRIO_DESCRIPTORS, RADEON_PRIO_BORDER_COLORS, -RADEON_PRIO_SAMPLER_BUFFER = 28, +RADEON_PRIO_SAMPLER_BUFFER = 20, RADEON_PRIO_VERTEX_BUFFER, -RADEON_PRIO_SHADER_RW_BUFFER = 32, +RADEON_PRIO_SHADER_RW_BUFFER = 24, RADEON_PRIO_SCRATCH_BUFFER, RADEON_PRIO_COMPUTE_GLOBAL, -RADEON_PRIO_SAMPLER_TEXTURE = 36, +RADEON_PRIO_SAMPLER_TEXTURE = 28, RADEON_PRIO_SHADER_RW_IMAGE, -RADEON_PRIO_SAMPLER_TEXTURE_MSAA = 40, +RADEON_PRIO_SAMPLER_TEXTURE_MSAA = 32, -RADEON_PRIO_COLOR_BUFFER = 44, +RADEON_PRIO_COLOR_BUFFER = 36, -RADEON_PRIO_DEPTH_BUFFER = 48, +RADEON_PRIO_DEPTH_BUFFER = 40, -RADEON_PRIO_COLOR_BUFFER_MSAA = 52, +RADEON_PRIO_COLOR_BUFFER_MSAA = 44, -RADEON_PRIO_DEPTH_BUFFER_MSAA = 56, +RADEON_PRIO_DEPTH_BUFFER_MSAA = 48, -RADEON_PRIO_CMASK = 60, +RADEON_PRIO_CMASK = 52, RADEON_PRIO_DCC, RADEON_PRIO_HTILE, RADEON_PRIO_SHADER_RINGS, /* 63 is the maximum value */ }; struct winsys_handle; struct radeon_winsys_ctx; struct radeon_winsys_cs_chunk { -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 97307] glsl/glcpp/tests/glcpp-test regression
https://bugs.freedesktop.org/show_bug.cgi?id=97307 Bug ID: 97307 Summary: glsl/glcpp/tests/glcpp-test regression Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: 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: i...@freedesktop.org, t_arc...@yahoo.com.au mesa: 17f1c49b9ad05af4f6482f6fa950e5dcc1a779d1 (master 12.1.0-devel) Testing ./glsl/glcpp/tests/120-undef-builtin.c... > src/compiler/glsl/glcpp/tests/120-undef-builtin.c.out (./glsl/glcpp/tests/120-undef-builtin.c.expected) FAIL --- ./glsl/glcpp/tests/120-undef-builtin.c.expected 2016-01-26 09:59:33.017797268 -0800 +++ src/compiler/glsl/glcpp/tests/120-undef-builtin.c.out 2016-08-11 10:30:15.630875304 -0700 @@ -1,6 +1,3 @@ -0:1(1): preprocessor error: Built-in (pre-defined) macro names cannot be undefined. -0:2(1): preprocessor error: Built-in (pre-defined) macro names cannot be undefined. -0:3(1): preprocessor error: Built-in (pre-defined) macro names cannot be undefined. 50b49d242d702e4728329cc59f87d929963e7c53 is the first bad commit commit 50b49d242d702e4728329cc59f87d929963e7c53 Author: Ian Romanick Date: Tue Aug 9 14:32:24 2016 -0700 glcpp: Only disallow #undef of pre-defined macros on GLSL ES >= 3.00 shaders Section 3.4 (Preprocessor) of the GLSL ES 3.00 spec says: It is an error to undefine or to redefine a built-in (pre-defined) macro name. The GLSL ES 1.00 spec does not contain this text. Section 3.3 (Preprocessor) of the GLSL 1.30 spec says: #define and #undef functionality are defined as is standard for C++ preprocessors for macro definitions both with and without macro parameters. At least as far as I can tell GCC allow '#undef __FILE__'. Furthermore, there are desktop OpenGL conformance tests that expect '#undef __VERSION__' and '#undef GL_core_profile' to work. Fixes: GL45-CTS.shaders.preprocessor.definitions.undefine_version_vertex GL45-CTS.shaders.preprocessor.definitions.undefine_version_fragment GL45-CTS.shaders.preprocessor.definitions.undefine_core_profile_vertex GL45-CTS.shaders.preprocessor.definitions.undefine_core_profile_fragment Signed-off-by: Ian Romanick Reviewed-by: Timothy Arceri Cc: mesa-sta...@lists.freedesktop.org :04 04 fbe8d2e45f4f1e6755e70b6e6f8fa971701e3ae8 d7c77a29d8aa922a28948df34eca499077eaac45 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
[Mesa-dev] [PATCH 3/3] glsl: Fix invariant matching in GLSL 4.30 and GLSL ES 1.00.
Old languages (GLSL <= 4.20 and GLSL ES 1.00) require "invariant" to be specified on both inputs and outputs, and match when linking. New languages only allow outputs to be qualified as "invariant" and remove the "invariant must match" restriction when linking varyings (because no input can have that qualifier). Commit 426a50e2089b12d33f5c075aa5622f64076914a3 introduced the new behavior for ES 3.00. It also removed the "must match" restriction for ES 1.00 shaders, which I believe is incorrect. This patch adds that back, as well as making 4.30+ follow the new rules. Thanks to Qiankun Miao for noticing this discrepancy. Fixes a WebGL 2.0 conformance test when run in Chromium: https://www.khronos.org/registry/webgl/sdk/tests/deqp/data/gles3/shaders/qualification_order.html?webglVersion=2 Cc: mesa-sta...@lists.freedesktop.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96971 Signed-off-by: Kenneth Graunke --- src/compiler/glsl/glsl_parser.yy| 4 +++- src/compiler/glsl/link_varyings.cpp | 20 +++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy index 4ab9e14..4f4a83c 100644 --- a/src/compiler/glsl/glsl_parser.yy +++ b/src/compiler/glsl/glsl_parser.yy @@ -1774,8 +1774,10 @@ type_qualifier: * variables. As only outputs can be declared as invariant, an invariant * output from one shader stage will still match an input of a subsequent * stage without the input being declared as invariant." + * + * On the desktop side, this text first appears in GLSL 4.30. */ - if (state->es_shader && state->language_version >= 300 && $$.flags.q.in) + if (state->is_version(430, 300) && $$.flags.q.in) _mesa_glsl_error(&@1, state, "invariant qualifiers cannot be used with shader inputs"); } | interpolation_qualifier type_qualifier diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index b4799d2..1bce3e0 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -308,7 +308,25 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, return; } - if (!prog->IsES && input->data.invariant != output->data.invariant) { + /* The GLSL 4.30 and GLSL ES 3.00 specifications say: +* +*"As only outputs need be declared with invariant, an output from +* one shader stage will still match an input of a subsequent stage +* without the input being declared as invariant." +* +* while GLSL 4.20 says: +* +*"For variables leaving one shader and coming into another shader, +* the invariant keyword has to be used in both shaders, or a link +* error will result." +* +* and GLSL ES 1.00 section 4.6.4 "Invariance and Linking" says: +* +*"The invariance of varyings that are declared in both the vertex +* and fragment shaders must match." +*/ + if (input->data.invariant != output->data.invariant && + prog->Version < (prog->IsES ? 300 : 430)) { linker_error(prog, "%s shader output `%s' %s invariant qualifier, " "but %s shader input %s invariant qualifier\n", -- 2.9.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] glsl: Tidy stream handling in merge_qualifier().
The previous commit fixed xfb_buffer handling, which was largely copy and pasted from the stream handling. The difference is that stream was set in input_layout_mask, so it worked. However, that's totally rubbish: stream is only valid on geometry shader outputs. Presumably this was to hack around inout. Instead, apply the solution I used in the previous fix. Really, we just need to separate shader interface and parameter qualifier handling so this isn't a mess, but this patch at least tidies it slightly. Cc: Timothy Arceri Signed-off-by: Kenneth Graunke --- src/compiler/glsl/ast_type.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp index 248b647..cabc698 100644 --- a/src/compiler/glsl/ast_type.cpp +++ b/src/compiler/glsl/ast_type.cpp @@ -178,8 +178,6 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc, if (state->stage == MESA_SHADER_GEOMETRY) { allowed_duplicates_mask.flags.i |= stream_layout_mask.flags.i; - input_layout_mask.flags.i |= - stream_layout_mask.flags.i; } if (is_single_layout_merge && !state->has_enhanced_layouts() && @@ -229,7 +227,8 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc, if (q.flags.q.stream) { this->flags.q.stream = 1; this->stream = q.stream; - } else if (!this->flags.q.stream && this->flags.q.out) { + } else if (!this->flags.q.stream && this->flags.q.out && +!this->flags.q.in) { /* Assign default global stream value */ this->flags.q.stream = 1; this->stream = state->out_qualifier->stream; -- 2.9.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] glsl: Fix inout qualifier handling in GLSL 4.40.
inout variables have q.in and q.out set. We were trying to set xfb_buffer = 1 for shader output variables (and inadvertantly setting it on inout parameters, too). But input_layout_mask doesn't have xfb_buffer set, so it was seen as in invalid input qualifier. This meant that all 'inout' parameters were broken. Caught by running a WebGL conformance test in Chromium: https://www.khronos.org/registry/webgl/sdk/tests/deqp/data/gles3/shaders/qualification_order.html?webglVersion=2 Fixes Piglit's tests/spec/glsl-4.40/compiler/inout-parameter-qualifier. Cc: Timothy Arceri Signed-off-by: Kenneth Graunke --- src/compiler/glsl/ast_type.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp index ef573e7..248b647 100644 --- a/src/compiler/glsl/ast_type.cpp +++ b/src/compiler/glsl/ast_type.cpp @@ -242,7 +242,8 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc, if (q.flags.q.xfb_buffer) { this->flags.q.xfb_buffer = 1; this->xfb_buffer = q.xfb_buffer; - } else if (!this->flags.q.xfb_buffer && this->flags.q.out) { + } else if (!this->flags.q.xfb_buffer && this->flags.q.out && +!this->flags.q.in) { /* Assign global xfb_buffer value */ this->flags.q.xfb_buffer = 1; this->xfb_buffer = state->out_qualifier->xfb_buffer; -- 2.9.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/32] genxml/gen6: Add a Surface Base Address field to HIER_DEPTH_BUFFER
--- src/intel/genxml/gen6.xml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml index 0f2be24..6ece749 100644 --- a/src/intel/genxml/gen6.xml +++ b/src/intel/genxml/gen6.xml @@ -1007,14 +1007,15 @@ - + - + + -- 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/32] genxml/gen6: Add uint MOCS fields for most things
--- src/intel/genxml/gen6.xml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml index cf94efc..0f2be24 100644 --- a/src/intel/genxml/gen6.xml +++ b/src/intel/genxml/gen6.xml @@ -1883,20 +1883,26 @@ + + + + + + -- 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 03/32] genxml/gen6: Fix the length of 3DSTATE_WM
--- src/intel/genxml/gen6.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml index 6ece749..1be0ede 100644 --- a/src/intel/genxml/gen6.xml +++ b/src/intel/genxml/gen6.xml @@ -1365,12 +1365,12 @@ - + - + -- 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 00/32] i965/blorp: Use genxml for state setup
This patch series is the next on the way towards generalizing blorp for usin in Vulkan. At this point, I'd say the project is about 80% complete. This series only applies on top of the last two blorp isl series which are still undergoing review. Patches 1-11 make minor genxml improvements. Some of this is to make gen6 better and more like gen7-9. The others are just general improvements that made converting blorp easier. Patches 12-14 make a few minor changes to i965 so that we can include brw_state.h and brw_context.h without brw_defines.h. Patches 15-23 re-arrange the state setup code in blorp to make the different gens emit the packets more-or-less in the same order. This was very helpful when trying to debug problems when switching to genxml because there should not be substantial changes in the batch between patch 23 and the end of the series. Patches 24-32 convert all of the blorp state setup to using genxml in a single file that gets recompiled 5 times, one for each gen. I'm extremely happy with the end result where almost all of the code is shared across gens. The one exception is WM and SF setup which is so different that we really need different code for gen6, gen7, and gen8. This series can be found here: https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=review/blorp-genxml Cc: Topi Pohjolainen Jason Ekstrand (32): genxml/gen6: Add uint MOCS fields for most things genxml/gen6: Add a Surface Base Address field to HIER_DEPTH_BUFFER genxml/gen6: Fix the length of 3DSTATE_WM genxml/gen6: Add the 3D_Prim_Topo_Type enum genxml/gen6: Make "Depth Clear Value" a uint genxml: Add a uint MOCS field to DEPTH_BUFFER packets genxml/gen6: Make SAMPLER_STATE look a bit more like gen7 genxml: Make VERTEX_ELEMENT_STATE::Valid a bool genxml: Make a couple of VERTEX_BUFFER_STATE fields boolean genxml: Add a uint MOCS field to VERTEX_BUFFER_STATE genxml/gen9: Make 3DSTATE_SBE::AttributeActiveComponentFormat an array i965/state: Move is_drawing_lines/points to gen6_clip_state.c i965: Stop including brw_defines.h in brw_state.h i965: Roll intel_reg.h into brw_defines.h i965/blorp: Make gen6 VS and GS disable helpers static i965/blorp: Move the non-static blorp state setup helpers to another file i965/blorp: Don't clear an empty region i965/blorp/gen6: Move constant disables higher up i965/blorp/gen7-8: Emit depth stencil state with CC and BLEND i965/blorp/gen6-7: Move surfaces and samplers closer together i965/blorp/gen6-7: Move multisample setup to right after samplers i965/blorp/gen8: Move viewport setup to after wm state i965/blorp: Stop setting point and line rasterization rules i965/blorp/gen6: Use genxml packing structs for state setup i965: Move gen6_blorp.c to a file that gets recompiled per-gen i965/blorp: Add genxml-based dynamic state emit functions i965/blorp: Add genxml-based sampler state emit function i965/blorp: Add a helper for emitting surface states i965/blorp: Add genxml-based vertex setup helpers i965/blorp: Use genxml for gen7 state setup i965/blorp: Use genxml for gen8-9 state setup i965/blorp: Remove no longer used state setup helpers src/intel/genxml/gen6.xml | 93 +- src/intel/genxml/gen7.xml |9 +- src/intel/genxml/gen75.xml |9 +- src/intel/genxml/gen8.xml |7 +- src/intel/genxml/gen9.xml | 42 +- src/intel/vulkan/genX_pipeline_util.h | 38 +- src/mesa/drivers/dri/i965/Makefile.am | 31 +- src/mesa/drivers/dri/i965/Makefile.sources | 19 +- src/mesa/drivers/dri/i965/blorp.c |9 +- src/mesa/drivers/dri/i965/blorp_priv.h | 94 +- src/mesa/drivers/dri/i965/brw_blorp.c |4 + src/mesa/drivers/dri/i965/brw_defines.h| 273 ++ src/mesa/drivers/dri/i965/brw_meta_util.c |1 + src/mesa/drivers/dri/i965/brw_pipe_control.c |2 +- src/mesa/drivers/dri/i965/brw_queryobj.c |1 - src/mesa/drivers/dri/i965/brw_state.h | 56 +- src/mesa/drivers/dri/i965/gen6_blorp.c | 965 - src/mesa/drivers/dri/i965/gen6_clip_state.c| 54 +- src/mesa/drivers/dri/i965/gen6_queryobj.c |1 - src/mesa/drivers/dri/i965/gen6_sf_state.c |4 +- src/mesa/drivers/dri/i965/gen7_blorp.c | 708 --- src/mesa/drivers/dri/i965/gen7_sf_state.c |2 +- src/mesa/drivers/dri/i965/gen8_blorp.c | 578 - src/mesa/drivers/dri/i965/genX_blorp_exec.c| 1103 src/mesa/drivers/dri/i965/hsw_queryobj.c |1 - src/mesa/drivers/dri/i965/intel_batchbuffer.c |1 - src/mesa/drivers/dri/i965/intel_batchbuffer.h |1 - src/mesa/drivers/dri/i965/intel_blit.c |1 - src/mesa/drivers/dri/i965/intel_extensions.c |1 + src/mesa/drivers/dri/i965/intel_fbo.c |1 + src/mesa/d
[Mesa-dev] [PATCH 05/32] genxml/gen6: Make "Depth Clear Value" a uint
The actual data storred is in float, UNORM24, or UNORM16 depending on the actual depth format. --- src/intel/genxml/gen6.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml index ae317a13..06b441b 100644 --- a/src/intel/genxml/gen6.xml +++ b/src/intel/genxml/gen6.xml @@ -797,7 +797,7 @@ - + -- 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 06/32] genxml: Add a uint MOCS field to DEPTH_BUFFER packets
This is easier than dealing with structs all the time --- src/intel/genxml/gen6.xml | 2 ++ src/intel/genxml/gen7.xml | 2 ++ src/intel/genxml/gen75.xml | 2 ++ src/intel/genxml/gen8.xml | 2 ++ src/intel/genxml/gen9.xml | 2 ++ 5 files changed, 10 insertions(+) diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml index 06b441b..3fe7377 100644 --- a/src/intel/genxml/gen6.xml +++ b/src/intel/genxml/gen6.xml @@ -958,6 +958,7 @@ + @@ -1038,6 +1039,7 @@ + diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml index 5e82fab..7e34fd6 100644 --- a/src/intel/genxml/gen7.xml +++ b/src/intel/genxml/gen7.xml @@ -1068,6 +1068,7 @@ + @@ -1198,6 +1199,7 @@ + diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml index f1be2f84..f0ad177 100644 --- a/src/intel/genxml/gen75.xml +++ b/src/intel/genxml/gen75.xml @@ -1177,6 +1177,7 @@ + @@ -1410,6 +1411,7 @@ + diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml index 8145ddd..4ed23db 100644 --- a/src/intel/genxml/gen8.xml +++ b/src/intel/genxml/gen8.xml @@ -1216,6 +1216,7 @@ + @@ -1452,6 +1453,7 @@ + diff --git a/src/intel/genxml/gen9.xml b/src/intel/genxml/gen9.xml index 1838d2c..9361edf 100644 --- a/src/intel/genxml/gen9.xml +++ b/src/intel/genxml/gen9.xml @@ -1266,6 +1266,7 @@ + @@ -1555,6 +1556,7 @@ + -- 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/32] genxml/gen6: Add the 3D_Prim_Topo_Type enum
--- src/intel/genxml/gen6.xml | 24 1 file changed, 24 insertions(+) diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml index 1be0ede..ae317a13 100644 --- a/src/intel/genxml/gen6.xml +++ b/src/intel/genxml/gen6.xml @@ -447,6 +447,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + -- 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 11/32] genxml/gen9: Make 3DSTATE_SBE::AttributeActiveComponentFormat an array
--- src/intel/genxml/gen9.xml | 35 +++- src/intel/vulkan/genX_pipeline_util.h | 38 +++ 2 files changed, 6 insertions(+), 67 deletions(-) diff --git a/src/intel/genxml/gen9.xml b/src/intel/genxml/gen9.xml index 51a2420..547d47f 100644 --- a/src/intel/genxml/gen9.xml +++ b/src/intel/genxml/gen9.xml @@ -2097,38 +2097,9 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + diff --git a/src/intel/vulkan/genX_pipeline_util.h b/src/intel/vulkan/genX_pipeline_util.h index d3b15d9..0e6dcd1 100644 --- a/src/intel/vulkan/genX_pipeline_util.h +++ b/src/intel/vulkan/genX_pipeline_util.h @@ -236,44 +236,12 @@ emit_3dstate_sbe(struct anv_pipeline *pipeline) .PointSpriteTextureCoordinateOrigin = UPPERLEFT, .NumberofSFOutputAttributes = wm_prog_data->num_varying_inputs, .ConstantInterpolationEnable = wm_prog_data->flat_inputs, + }; #if GEN_GEN >= 9 - .Attribute0ActiveComponentFormat = ACF_XYZW, - .Attribute1ActiveComponentFormat = ACF_XYZW, - .Attribute2ActiveComponentFormat = ACF_XYZW, - .Attribute3ActiveComponentFormat = ACF_XYZW, - .Attribute4ActiveComponentFormat = ACF_XYZW, - .Attribute5ActiveComponentFormat = ACF_XYZW, - .Attribute6ActiveComponentFormat = ACF_XYZW, - .Attribute7ActiveComponentFormat = ACF_XYZW, - .Attribute8ActiveComponentFormat = ACF_XYZW, - .Attribute9ActiveComponentFormat = ACF_XYZW, - .Attribute10ActiveComponentFormat = ACF_XYZW, - .Attribute11ActiveComponentFormat = ACF_XYZW, - .Attribute12ActiveComponentFormat = ACF_XYZW, - .Attribute13ActiveComponentFormat = ACF_XYZW, - .Attribute14ActiveComponentFormat = ACF_XYZW, - .Attribute15ActiveComponentFormat = ACF_XYZW, - /* wow, much field, very attribute */ - .Attribute16ActiveComponentFormat = ACF_XYZW, - .Attribute17ActiveComponentFormat = ACF_XYZW, - .Attribute18ActiveComponentFormat = ACF_XYZW, - .Attribute19ActiveComponentFormat = ACF_XYZW, - .Attribute20ActiveComponentFormat = ACF_XYZW, - .Attribute21ActiveComponentFormat = ACF_XYZW, - .Attribute22ActiveComponentFormat = ACF_XYZW, - .Attribute23ActiveComponentFormat = ACF_XYZW, - .Attribute24ActiveComponentFormat = ACF_XYZW, - .Attribute25ActiveComponentFormat = ACF_XYZW, - .Attribute26ActiveComponentFormat = ACF_XYZW, - .Attribute27ActiveComponentFormat = ACF_XYZW, - .Attribute28ActiveComponentFormat = ACF_XYZW, - .Attribute29ActiveComponentFormat = ACF_XYZW, - .Attribute28ActiveComponentFormat = ACF_XYZW, - .Attribute29ActiveComponentFormat = ACF_XYZW, - .Attribute30ActiveComponentFormat = ACF_XYZW, + for (unsigned i = 0; i < 32; i++) + sbe.AttributeActiveComponentFormat[i] = ACF_XYZW; #endif - }; #if GEN_GEN >= 8 /* On Broadwell, they broke 3DSTATE_SBE into two packets */ -- 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 21/32] i965/blorp/gen6-7: Move multisample setup to right after samplers
This mimics gen8 blorp --- src/mesa/drivers/dri/i965/gen6_blorp.c | 9 + src/mesa/drivers/dri/i965/gen7_blorp.c | 10 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.c b/src/mesa/drivers/dri/i965/gen6_blorp.c index b252d78..7d37ea3 100644 --- a/src/mesa/drivers/dri/i965/gen6_blorp.c +++ b/src/mesa/drivers/dri/i965/gen6_blorp.c @@ -515,10 +515,6 @@ gen6_blorp_exec(struct brw_context *brw, brw_upload_state_base_address(brw); - gen6_emit_3dstate_multisample(brw, params->dst.surf.samples); - gen6_emit_3dstate_sample_mask(brw, - params->dst.surf.samples > 1 ? - (1 << params->dst.surf.samples) - 1 : 1); gen6_blorp_emit_vertices(brw, params); gen6_blorp_emit_urb_config(brw, params); if (params->wm_prog_data) { @@ -559,6 +555,11 @@ gen6_blorp_exec(struct brw_context *brw, gen6_blorp_emit_sampler_state_pointers(brw, sampler_offset); } + gen6_emit_3dstate_multisample(brw, params->dst.surf.samples); + gen6_emit_3dstate_sample_mask(brw, + params->dst.surf.samples > 1 ? + (1 << params->dst.surf.samples) - 1 : 1); + gen6_blorp_emit_vs_disable(brw, params); gen6_blorp_emit_gs_disable(brw, params); gen6_blorp_emit_clip_disable(brw); diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.c b/src/mesa/drivers/dri/i965/gen7_blorp.c index adbf909..1b9c853 100644 --- a/src/mesa/drivers/dri/i965/gen7_blorp.c +++ b/src/mesa/drivers/dri/i965/gen7_blorp.c @@ -468,11 +468,6 @@ gen7_blorp_exec(struct brw_context *brw, uint32_t wm_bind_bo_offset = 0; brw_upload_state_base_address(brw); - - gen6_emit_3dstate_multisample(brw, params->dst.surf.samples); - gen6_emit_3dstate_sample_mask(brw, - params->dst.surf.samples > 1 ? - (1 << params->dst.surf.samples) - 1 : 1); gen6_blorp_emit_vertices(brw, params); gen7_blorp_emit_urb_config(brw, params); if (params->wm_prog_data) { @@ -520,6 +515,11 @@ gen7_blorp_exec(struct brw_context *brw, gen7_blorp_emit_sampler_state_pointers_ps(brw, sampler_offset); } + gen6_emit_3dstate_multisample(brw, params->dst.surf.samples); + gen6_emit_3dstate_sample_mask(brw, + params->dst.surf.samples > 1 ? + (1 << params->dst.surf.samples) - 1 : 1); + gen7_blorp_emit_vs_disable(brw); gen7_blorp_emit_hs_disable(brw); gen7_blorp_emit_te_disable(brw); -- 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/32] genxml/gen6: Make SAMPLER_STATE look a bit more like gen7
--- src/intel/genxml/gen6.xml | 45 ++--- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml index 3fe7377..2499b46 100644 --- a/src/intel/genxml/gen6.xml +++ b/src/intel/genxml/gen6.xml @@ -414,14 +414,7 @@ - - - - - - - - + @@ -434,17 +427,22 @@ - - - - - - - - - - - + + + + + + + + + + + + + + + + @@ -717,6 +715,15 @@ + + + + + + + + + -- 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 08/32] genxml: Make VERTEX_ELEMENT_STATE::Valid a bool
--- src/intel/genxml/gen6.xml | 2 +- src/intel/genxml/gen7.xml | 2 +- src/intel/genxml/gen75.xml | 2 +- src/intel/genxml/gen8.xml | 2 +- src/intel/genxml/gen9.xml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml index 2499b46..a281a1a 100644 --- a/src/intel/genxml/gen6.xml +++ b/src/intel/genxml/gen6.xml @@ -21,7 +21,7 @@ - + diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml index 7e34fd6..3fe47c7 100644 --- a/src/intel/genxml/gen7.xml +++ b/src/intel/genxml/gen7.xml @@ -35,7 +35,7 @@ - + diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml index f0ad177..63449a9 100644 --- a/src/intel/genxml/gen75.xml +++ b/src/intel/genxml/gen75.xml @@ -45,7 +45,7 @@ - + diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml index 4ed23db..828ad90 100644 --- a/src/intel/genxml/gen8.xml +++ b/src/intel/genxml/gen8.xml @@ -49,7 +49,7 @@ - + diff --git a/src/intel/genxml/gen9.xml b/src/intel/genxml/gen9.xml index 9361edf..b72f9bf 100644 --- a/src/intel/genxml/gen9.xml +++ b/src/intel/genxml/gen9.xml @@ -37,7 +37,7 @@ - + -- 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/32] genxml: Make a couple of VERTEX_BUFFER_STATE fields boolean
--- src/intel/genxml/gen6.xml | 2 +- src/intel/genxml/gen7.xml | 4 ++-- src/intel/genxml/gen75.xml | 4 ++-- src/intel/genxml/gen8.xml | 2 +- src/intel/genxml/gen9.xml | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml index a281a1a..6241665 100644 --- a/src/intel/genxml/gen6.xml +++ b/src/intel/genxml/gen6.xml @@ -12,7 +12,7 @@ - + diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml index 3fe47c7..adf764e 100644 --- a/src/intel/genxml/gen7.xml +++ b/src/intel/genxml/gen7.xml @@ -24,9 +24,9 @@ - + - + diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml index 63449a9..b93e920 100644 --- a/src/intel/genxml/gen75.xml +++ b/src/intel/genxml/gen75.xml @@ -34,9 +34,9 @@ - + - + diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml index 828ad90..d460c48 100644 --- a/src/intel/genxml/gen8.xml +++ b/src/intel/genxml/gen8.xml @@ -40,7 +40,7 @@ - + diff --git a/src/intel/genxml/gen9.xml b/src/intel/genxml/gen9.xml index b72f9bf..4837b3c 100644 --- a/src/intel/genxml/gen9.xml +++ b/src/intel/genxml/gen9.xml @@ -28,7 +28,7 @@ - + -- 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 17/32] i965/blorp: Don't clear an empty region
--- src/mesa/drivers/dri/i965/brw_blorp.c | 4 1 file changed, 4 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c index f12c1f8..ccc7136 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp.c +++ b/src/mesa/drivers/dri/i965/brw_blorp.c @@ -582,6 +582,10 @@ do_single_blorp_clear(struct brw_context *brw, struct gl_framebuffer *fb, y1 = rb->Height - fb->_Ymin; } + /* If the clear region is empty, just return. */ + if (x0 == x1 || y0 == y1) + return true; + bool can_fast_clear = !partial_clear; bool color_write_disable[4] = { false, false, false, false }; -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev