Re: [Mesa-dev] [PATCH 4/4] nir: detect more induction variables
Den ons. 28. nov. 2018 kl. 04:26 skrev Timothy Arceri : > > This adds allows loop analysis to detect inductions varibales that > are incremented in both branches of an if rather than in a main > loop block. For example: > >loop { > block block_1: > /* preds: block_0 block_7 */ > vec1 32 ssa_8 = phi block_0: ssa_4, block_7: ssa_20 > vec1 32 ssa_9 = phi block_0: ssa_0, block_7: ssa_4 > vec1 32 ssa_10 = phi block_0: ssa_1, block_7: ssa_4 > vec1 32 ssa_11 = phi block_0: ssa_2, block_7: ssa_21 > vec1 32 ssa_12 = phi block_0: ssa_3, block_7: ssa_22 > vec4 32 ssa_13 = vec4 ssa_12, ssa_11, ssa_10, ssa_9 > vec1 32 ssa_14 = ige ssa_8, ssa_5 > /* succs: block_2 block_3 */ > if ssa_14 { > block block_2: > /* preds: block_1 */ > break > /* succs: block_8 */ > } else { > block block_3: > /* preds: block_1 */ > /* succs: block_4 */ > } > block block_4: > /* preds: block_3 */ > vec1 32 ssa_15 = ilt ssa_6, ssa_8 > /* succs: block_5 block_6 */ > if ssa_15 { > block block_5: > /* preds: block_4 */ > vec1 32 ssa_16 = iadd ssa_8, ssa_7 > vec1 32 ssa_17 = load_const (0x3f80 /* 1.00*/) > /* succs: block_7 */ > } else { > block block_6: > /* preds: block_4 */ > vec1 32 ssa_18 = iadd ssa_8, ssa_7 > vec1 32 ssa_19 = load_const (0x3f80 /* 1.00*/) > /* succs: block_7 */ > } > block block_7: > /* preds: block_5 block_6 */ > vec1 32 ssa_20 = phi block_5: ssa_16, block_6: ssa_18 > vec1 32 ssa_21 = phi block_5: ssa_17, block_6: ssa_4 > vec1 32 ssa_22 = phi block_5: ssa_4, block_6: ssa_19 > /* succs: block_1 */ >} > > Unfortunatly GCM could move the addition out of the if for us > (making this patch unrequired) but we still cannot enable the GCM > pass without regressions. > Just some questions / suggestions from my side for now. I'll try to take a closer look at the patch later today. While GCM would be nice, to me it seems that adding an if-opt instead, that pulls common code from both branches of an if out of the if on a more general basis, would get us this, plus a bunch of other benefits? As far as I can see there should never be negative impacts from pulling common code out like that, but I might be wrong. Did you look into that? I bet out did, I'm just interested in how that worked out. Since GCM is not yet where we want it to be, maybe we'd want to implement LICM? That obviously does not come into play with what this patch adresses, but it might help get a more accurate estimate of the cost/benefit of unrolling? (Invariant computations that will be CSE'd will not be counted multiple times). This might already be accounted for by counting the invariant computations only once? > This unrolls a loop in Rise of The Tomb Raider. > > vkpipeline-db results (VEGA): > > Totals from affected shaders: > SGPRS: 88 -> 96 (9.09 %) > VGPRS: 56 -> 52 (-7.14 %) > Spilled SGPRs: 0 -> 0 (0.00 %) > Spilled VGPRs: 0 -> 0 (0.00 %) > Private memory VGPRs: 0 -> 0 (0.00 %) > Scratch size: 0 -> 0 (0.00 %) dwords per thread > Code Size: 2168 -> 4560 (110.33 %) bytes > LDS: 0 -> 0 (0.00 %) blocks > Max Waves: 4 -> 4 (0.00 %) > Wait states: 0 -> 0 (0.00 %) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32211 > --- > src/compiler/nir/nir_loop_analyze.c | 36 + > 1 file changed, 36 insertions(+) > > diff --git a/src/compiler/nir/nir_loop_analyze.c > b/src/compiler/nir/nir_loop_analyze.c > index 8903e15105..cf97d6bf06 100644 > --- a/src/compiler/nir/nir_loop_analyze.c > +++ b/src/compiler/nir/nir_loop_analyze.c > @@ -245,6 +245,42 @@ compute_induction_information(loop_info_state *state) > if (src_var->in_if_branch || src_var->in_nested_loop) > break; > > + /* Detect inductions varibales that are incremented in both branches > + * of an unnested if rather than in a loop block. > + */ > + if (is_var_phi(src_var)) { > +nir_phi_instr *src_phi = > + nir_instr_as_phi(src_var->def->parent_instr); > + > +nir_op alu_op; > +nir_ssa_def *alu_srcs[2] = {0}; > +nir_foreach_phi_src(src2, src_phi) { > + nir_loop_variable *src_var2 = > + get_loop_var(src2->src.ssa, state); > + > + if (!src_var2->in_if_branch || !is_var_alu(src_var2)) > + break; > + > + nir_alu_instr *alu = > + nir_instr_as_alu(src_var2->def->parent_instr); > + if (nir_op_infos[alu->op].num_inputs != 2) > + break; > + > + if (alu->src[0].src.ssa == alu_srcs[0] && > + alu->src[1].src.ssa == alu_srcs[1] && > + alu->op == alu_op) { > + /*
Re: [Mesa-dev] [PATCH] intel/compiler: Use nir's info when checking uses_streams.
Reviewed-by: Iago Toral Quiroga On Tue, 2018-11-27 at 15:34 -0800, Kenneth Graunke wrote: > Vulkan and Gallium don't use Mesa's gl_program data structure, so > they > can't poke at 'prog'. But we can simply use the copy of the shader > info > stored with the NIR shader, which is guaranteed to exist. > > Reviewed-by: Jason Ekstrand > --- > src/intel/compiler/brw_vec4_gs_visitor.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/intel/compiler/brw_vec4_gs_visitor.cpp > b/src/intel/compiler/brw_vec4_gs_visitor.cpp > index 63ff27e5e08..a6e38b0f379 100644 > --- a/src/intel/compiler/brw_vec4_gs_visitor.cpp > +++ b/src/intel/compiler/brw_vec4_gs_visitor.cpp > @@ -667,7 +667,7 @@ brw_compile_gs(const struct brw_compiler > *compiler, void *log_data, > prog_data->control_data_format = > GEN7_GS_CONTROL_DATA_FORMAT_GSCTL_SID; > > /* We only have to emit control bits if we are using > streams */ > - if (prog && prog->info.gs.uses_streams) > + if (shader->info.gs.uses_streams) > c.control_data_bits_per_vertex = 2; > else > c.control_data_bits_per_vertex = 0; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] docs: Document and *require* usage of Signed-off-by
This adds the "Developer's Certificate of Origin 1.1" from the Linux kernel. It indicates that by using Signed-off-by you are certifying that your patch meets the DCO 1.1 guidelines. It also changes Signed-off-by from being optional to being required. Signed-off-by: Jordan Justen Cc: Matt Turner --- docs/submittingpatches.html | 52 - 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html index 9ae750d5a15..6d506b3691b 100644 --- a/docs/submittingpatches.html +++ b/docs/submittingpatches.html @@ -20,6 +20,8 @@ Basic guidelines Patch formatting +Patch Signing (Signed-off-by, Developer's + Certificate of Origin) Testing Patches Mailing Patches Reviewing Patches @@ -73,7 +75,9 @@ if needed. For example: is necessary, and removing it causes no regressions in piglit on any platform. -A "Signed-off-by:" line is not required, but not discouraged either. +A "Signed-off-by:" line is required. The format +and meaning of Signed-off-by is documented below in +the patch signing section. If a patch addresses a bugzilla issue, that should be noted in the patch comment. For example: @@ -129,7 +133,53 @@ Please use common sense and do not blindly add everyone. + + Patch Signing (Signed-off-by, Developer's Certificate of Origin) + + + As described in the patch formatting + section, you must sign your patch by including Signed-off-by in the + patch commit message. The Signed-off-by must include your real name + and email address in this format: + + + Signed-off-by: Joe Hacker jhac...@foo.com + + + By adding Signed-of-by to your contributed patch, you certify that + your contribution meets the guidelines of the Developer's + Certificate of Origin: + + + +Developer's Certificate of Origin 1.1 +- + +By making a contribution to this project, I certify that: + + (a) The contribution was created in whole or in part by me and I + have the right to submit it under the open source license + indicated in the file; or + + (b) The contribution is based upon previous work that, to the best + of my knowledge, is covered under an appropriate open source + license and I have the right under that license to submit that + work with modifications, whether created in whole or in part + by me, under the same open source license (unless I am + permitted to submit under a different license), as indicated + in the file; or + + (c) The contribution was provided directly to me by some other + person who certified (a), (b) or (c) and I have not modified + it. + + (d) I understand and agree that this project and the contribution + are public and that a record of the contribution (including all + personal information I submit with it, including my sign-off) is + maintained indefinitely and may be redistributed consistent with + this project or the open source license(s) involved. + Testing Patches -- 2.20.0.rc1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: define NULL in egldevice.h
Reviewed-by: Matt Turner I'll commit it tomorrow. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965: Set the FBO error state INCOMPLETE_ATTACHMENT only for SRGB_R8
On 11/22/18 8:00 PM, Gert Wollny wrote: Originally the driver reported GL_FRAMEBUFFER_UNSUPPORTED in all cases, adding more specific error messages was not correct and broke many tests. Mostly revert this and only report GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT for MESA_FORMAT_R_SRGB8. Fixes: ebcde3454552adc6d3fea8af2207aafaba857796 i965: be more specific about FBO completeness errors Like with patch 1, fix 'Fixes', with that Reviewed-by: Tapani Pälli Signed-off-by: Gert Wollny --- src/mesa/drivers/dri/i965/intel_fbo.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c index 7e40d61a47..5bcd846a1b 100644 --- a/src/mesa/drivers/dri/i965/intel_fbo.c +++ b/src/mesa/drivers/dri/i965/intel_fbo.c @@ -719,7 +719,7 @@ intel_validate_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb) "FBO incomplete: separate stencil unsupported\n"); } if (stencil_mt->format != MESA_FORMAT_S_UINT8) { - fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT, + fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED, "FBO incomplete: separate stencil is %s " "instead of S8\n", _mesa_get_format_name(stencil_mt->format)); @@ -750,7 +750,7 @@ intel_validate_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb) */ rb = fb->Attachment[i].Renderbuffer; if (rb == NULL) { -fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT, +fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED, "FBO incomplete: attachment without " "renderbuffer\n"); continue; @@ -771,8 +771,15 @@ intel_validate_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb) continue; } + if (rb->Format == MESA_FORMAT_R_SRGB8) { +fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT, + "FBO incomplete: Format not color renderable: %s\n", + _mesa_get_format_name(rb->Format)); +continue; + } + if (!brw_render_target_supported(brw, rb)) { -fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT, +fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED, "FBO incomplete: Unsupported HW " "texture/renderbuffer format attached: %s\n", _mesa_get_format_name(intel_rb_format(irb))); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] i965: Explicitely handle swizzles for MESA_FORMAT_R_SRGB8
On 11/22/18 8:00 PM, Gert Wollny wrote: The format is emulated by using ISL_FORMAT_L8_SRGB, therefore we need to force swizzles for the GBA channels. However, doing this only based on the data type GL_RED breaks other formats, therefore, test specifically for the format. Fixes: 5363869d4971780401b21bb75083ef2518c12be 965: Force zero swizzles for unused components in GL_RED and GL_RG As Emil said, 'Fixes' line needs to be fixed before committing. Otherwise this LGTM; Reviewed-by: Tapani Pälli Signed-off-by: Gert Wollny --- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 018bae98e8..4daa0e2add 100644 --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c @@ -420,11 +420,15 @@ brw_get_texture_swizzle(const struct gl_context *ctx, } break; case GL_RED: - swizzles[1] = SWIZZLE_ZERO; + if (img->TexFormat == MESA_FORMAT_R_SRGB8) { + swizzles[0] = SWIZZLE_X; + swizzles[1] = SWIZZLE_ZERO; + swizzles[2] = SWIZZLE_ZERO; + swizzles[3] = SWIZZLE_ONE; + break; + } /* fallthrough */ case GL_RG: - swizzles[2] = SWIZZLE_ZERO; - /* fallthrough */ case GL_RGB: if (_mesa_get_format_bits(img->TexFormat, GL_ALPHA_BITS) > 0 || img->TexFormat == MESA_FORMAT_RGB_DXT1 || ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] gallivm: Use nextafterf(0.5, 0.0) as rounding constant
The common truncf(x + 0.5) fails for the floating-point value just less than 0.5 (nextafterf(0.5, 0.0)). nextafterf(0.5, 0.0) + 0.5, after rounding is 1.0, thus truncf does not produce the desired value. The solution is to add nextafterf(0.5, 0.0) instead of 0.5 before truncating. This works for all values. --- I noticed this while investigating https://bugs.gentoo.org/665570 but it does not fix it. Roland, do you have a suggestion for how to make lp_build_iround() work on non-SSE/non-Altivec platforms? I notice that if I unconditionally return TRUE from arch_rounding_available() and make lp_build_round_arch() take the SSE4.1 path (that emits llvm.nearbyint) it passes on ARM64. I noticed there's some hack in lp_test_arit.c:test_unary: if (test->ref == && length == 2 && ref != roundf(testval)) { /* FIXME: The generic (non SSE) path in lp_build_iround, which is * always taken for length==2 regardless of native round support, * does not round to even. */ expected_pass = FALSE; } It'd be nice to get rid of that.. but maybe we can somehow use it to just mark all the round tests as expected fail on other platforms if no real fix is forthcoming? src/gallium/auxiliary/gallivm/lp_bld_arit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c b/src/gallium/auxiliary/gallivm/lp_bld_arit.c index f348833206b..c050bfdb936 100644 --- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c +++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c @@ -2477,7 +2477,7 @@ lp_build_iround(struct lp_build_context *bld, else { LLVMValueRef half; - half = lp_build_const_vec(bld->gallivm, type, 0.5); + half = lp_build_const_vec(bld->gallivm, type, nextafterf(0.5, 0.0)); if (type.sign) { LLVMTypeRef vec_type = bld->vec_type; -- 2.18.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by
On 2018-11-27 19:53:38, Matt Turner wrote: > On Tue, Nov 27, 2018 at 7:26 PM Jordan Justen > wrote: > > On 2018-11-27 18:04:17, Matt Turner wrote: > > > By all means, require it (with a git hook) if you like. > > > > I personally don't want to push for that right now. > > > > I guess I would like it to be required someday, primarily because it > > creates a standard process for open source projects to use. (So, > > people are more likely to be used to it in general when contributing > > to open source projects.) > > > > But, I'm not confident that the consensus for Mesa would be in favor > > of making that change right now. So, as an alternative I'd like to > > remove any barriers (such as ambiguity) to its usage in Mesa. > > I don't think requiring S-o-b is onerous. Using S-o-b in a defined way > is just making explicit the current implicit promise to the community > that "I have the right to contribute this code" etc. I'm not sure why > you're hesitant. Ok. I'll type that version up. -Jordan ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] egl: define NULL in egldevice.h
Otherwise, I get this error: main/egldevice.h:54:13: error: ‘NULL’ undeclared (first use in this function) dev = NULL; ^~~~ with this config: ./autogen.sh --enable-gles1 --enable-gles2 --with-platforms='surfaceless' --disable-glx --with-dri-drivers="i965" --with-gallium-drivers="" --enable-gbm v3: Use stddef.h (Matt) --- src/egl/main/egldevice.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/egl/main/egldevice.h b/src/egl/main/egldevice.h index ddcdcd17f5..83a47d5eac 100644 --- a/src/egl/main/egldevice.h +++ b/src/egl/main/egldevice.h @@ -31,9 +31,9 @@ #include +#include #include "egltypedefs.h" - #ifdef __cplusplus extern "C" { #endif -- 2.18.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process
On Tue, Nov 27, 2018 at 7:45 PM Jordan Justen wrote: > > On 2018-11-27 19:20:09, Matt Turner wrote: > > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen > > wrote: > > > > > > This documents a mechanism for using GitLab Merge Requests as an > > > optional, secondary way to get code reviews for patchsets. > > > > > > We still require all patches to be emailed. > > > > > > Aside from the potential usage for code review comments, it might also > > > help reviewers to find unmerged patchsets which need review. (Assuming > > > it doesn't fall into the same fate of patchwork with hundreds of open > > > MRs.) > > > > > > Signed-off-by: Jordan Justen > > > Cc: Jason Ekstrand > > > --- > > > docs/submittingpatches.html | 25 + > > > 1 file changed, 25 insertions(+) > > > > > > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html > > > index 5d8ba443191..852f28c198a 100644 > > > --- a/docs/submittingpatches.html > > > +++ b/docs/submittingpatches.html > > > @@ -24,6 +24,7 @@ > > > Testing Patches > > > Mailing Patches > > > Reviewing Patches > > > +GitLab Merge Requests > > > Nominating a commit for a stable branch > > > Criteria for accepting patches to the stable > > > branch > > > Sending backports for the stable branch > > > @@ -282,6 +283,30 @@ which tells the patch author that the patch can be > > > committed, as long > > > as the issues are resolved first. > > > > > > > > > +GitLab Merge Requests > > > + > > > + > > > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > > > + Requests (MR) can be used as an optional, secondary method of > > > + obtaining code review for patches. > > > + > > > + > > > + > > > + All patches should be submitted using email as well > > > + Consider including a link to the MR in your email based > > > +cover-letter > > > + Address code review from both email and the MR > > > > Discussion point: I think attempting to have simultaneous review in > > two places is a recipe for wasted time. > > That's possible. It also happens on email sometimes. But, I want to > say that maybe the usual problem is too little code review, and not > too much? :) > > > Strawman: maybe we can only email the cover letter to the mailing > > list and include in it a link to the MR? > > I was hoping to make a smaller step and see what happens. Maybe this > will give people the chance to try out MR based reviews, but not take > away email based reviews just yet. > > I don't think we should move away from email based reviews until we > are sure MR's actually work better. (I'm far from convinced on this > point. :) That's fair. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by
On Tue, Nov 27, 2018 at 7:26 PM Jordan Justen wrote: > > On 2018-11-27 18:04:17, Matt Turner wrote: > > On Tue, Nov 27, 2018 at 6:00 PM Jordan Justen > > wrote: > > > > > > On 2018-11-27 17:17:15, Matt Turner wrote: > > > > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen > > > > wrote: > > > > > > > > > > This adds the "Developer's Certificate of Origin 1.1" from the Linux > > > > > kernel. It indicates that by using Signed-off-by you are certifying > > > > > that your patch meets the DCO 1.1 guidelines. > > > > > > > > Do we gain anything if it's optional? > > > > > > As I recall, one thing that bothered you about Signed-off-by in Mesa > > > is that it wasn't documented what it meant when it was used. > > > > > > Perhaps there are developers that don't want to use Signed-off-by with > > > an undocumented meaning for Mesa. If that is the case, then this might > > > help. I wasn't sure if you fell into that category. > > > > > > I use -s whenever I commit, so requiring it would not bother me. But, > > > I notice that many people (such as yourself) do not, so I didn't see > > > the need to push for that. > > > > > > If it's well documented, and becomes commonly used, then perhaps > > > requiring it might be a reasonable thing to consider. I won't be > > > holding my breath while waiting on that. :) > > > > I don't have a problem requiring it. I sign-off on commits I make to > > Gentoo, to Linux, etc. > > If it has the same meaning as with the Linux kernel, but is not > required, then you won't use it? > > I guess your concern might be that you are then giving something to > the project that others can choose not to. ? I'm not aware of any projects that define S-o-b but make it optional. I suspect there's a reason that those that define it require it. > > I'm just against cargo-culting it like we're doing now without a > > defined meaning. > > The purpose of this patch is to give it a defined meaning. And the > meaning I chose is the one that people are more likely to have in mind > when they use Signed-off-by. Maybe that's too big of an assumption on > my part, but I think several other open source projects have followed > the kernel on this. I understand, and agree. I'm just suggesting that it may not be good to do something different than all the other projects that meaningfully use S-o-b (i.e., by making it optional). > > By all means, require it (with a git hook) if you like. > > I personally don't want to push for that right now. > > I guess I would like it to be required someday, primarily because it > creates a standard process for open source projects to use. (So, > people are more likely to be used to it in general when contributing > to open source projects.) > > But, I'm not confident that the consensus for Mesa would be in favor > of making that change right now. So, as an alternative I'd like to > remove any barriers (such as ambiguity) to its usage in Mesa. I don't think requiring S-o-b is onerous. Using S-o-b in a defined way is just making explicit the current implicit promise to the community that "I have the right to contribute this code" etc. I'm not sure why you're hesitant. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] st/mesa: expose GL_OES_texture_view
On Tue, Nov 27, 2018 at 9:17 PM Ilia Mirkin wrote: > On Tue, Nov 27, 2018 at 9:00 PM Marek Olšák wrote: > > > > From: Marek Olšák > > > > For format fallbacks like ETC and ASTC, switching between sRGB and linear > > decoding is undefined, or at least is not bit-exact. Same as > > EXT_texture_sRGB_decode on GLES. > > > > There are no piglit or dEQP regresssions. > > Just checking -- did you run dEQP on hardware that uses the ASTC > fallbacks? If not, could you? > radeonsi doesn't support ASTC, so I always use the fallback. Like I said, no regressions in newly enabled tests. > > Also, what are the conditions for enabling ASTC fallbacks? Basically > the issue is that OES_texture_view requires ASTC, so I want to make > sure it'll be enabled for any drivers that > PIPE_CAP_SAMPLER_VIEW_TARGET is enabled for. > ASTC is always enabled. The existing behavior being talked about is already exposed by EXT_texture_sRGB_decode on GLES. The extension only adds another way to access that behavior. I'm open to discussing how to make EXT_texture_sRGB_decode bit-exact for compressed format fallbacks. I may also let it slide if test suites don't care. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process
On 2018-11-27 19:20:09, Matt Turner wrote: > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen > wrote: > > > > This documents a mechanism for using GitLab Merge Requests as an > > optional, secondary way to get code reviews for patchsets. > > > > We still require all patches to be emailed. > > > > Aside from the potential usage for code review comments, it might also > > help reviewers to find unmerged patchsets which need review. (Assuming > > it doesn't fall into the same fate of patchwork with hundreds of open > > MRs.) > > > > Signed-off-by: Jordan Justen > > Cc: Jason Ekstrand > > --- > > docs/submittingpatches.html | 25 + > > 1 file changed, 25 insertions(+) > > > > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html > > index 5d8ba443191..852f28c198a 100644 > > --- a/docs/submittingpatches.html > > +++ b/docs/submittingpatches.html > > @@ -24,6 +24,7 @@ > > Testing Patches > > Mailing Patches > > Reviewing Patches > > +GitLab Merge Requests > > Nominating a commit for a stable branch > > Criteria for accepting patches to the stable > > branch > > Sending backports for the stable branch > > @@ -282,6 +283,30 @@ which tells the patch author that the patch can be > > committed, as long > > as the issues are resolved first. > > > > > > +GitLab Merge Requests > > + > > + > > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > > + Requests (MR) can be used as an optional, secondary method of > > + obtaining code review for patches. > > + > > + > > + > > + All patches should be submitted using email as well > > + Consider including a link to the MR in your email based > > +cover-letter > > + Address code review from both email and the MR > > Discussion point: I think attempting to have simultaneous review in > two places is a recipe for wasted time. That's possible. It also happens on email sometimes. But, I want to say that maybe the usual problem is too little code review, and not too much? :) > Strawman: maybe we can only email the cover letter to the mailing > list and include in it a link to the MR? I was hoping to make a smaller step and see what happens. Maybe this will give people the chance to try out MR based reviews, but not take away email based reviews just yet. I don't think we should move away from email based reviews until we are sure MR's actually work better. (I'm far from convinced on this point. :) -Jordan ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: define NULL in egldevice.h
Wouldn't it be better to just include the stdlib.h/stddef.h header that defines NULL? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] egl: define NULL in egldevice.h
Otherwise, I get this error: main/egldevice.h:54:13: error: ‘NULL’ undeclared (first use in this function) dev = NULL; ^~~~ with this config: ./autogen.sh --enable-gles1 --enable-gles2 --with-platforms='surfaceless' --disable-glx --with-dri-drivers="i965" --with-gallium-drivers="" --enable-gbm v2: remove change id --- src/egl/main/egldevice.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/egl/main/egldevice.h b/src/egl/main/egldevice.h index ddcdcd17f5..f161942d26 100644 --- a/src/egl/main/egldevice.h +++ b/src/egl/main/egldevice.h @@ -33,6 +33,9 @@ #include #include "egltypedefs.h" +#ifndef NULL +#define NULL 0 +#endif #ifdef __cplusplus extern "C" { -- 2.18.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] egl: define NULL in egldevice.h
Otherwise, I get this error: main/egldevice.h:54:13: error: ‘NULL’ undeclared (first use in this function) dev = NULL; ^~~~ with this config: ./autogen.sh --enable-gles1 --enable-gles2 --with-platforms='surfaceless' --disable-glx --with-dri-drivers="i965" --with-gallium-drivers="" --enable-gbm Change-Id: I4332bfcfd19aecf239497591507aad921fffedf1 --- src/egl/main/egldevice.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/egl/main/egldevice.h b/src/egl/main/egldevice.h index ddcdcd17f5a..f161942d26f 100644 --- a/src/egl/main/egldevice.h +++ b/src/egl/main/egldevice.h @@ -33,6 +33,9 @@ #include #include "egltypedefs.h" +#ifndef NULL +#define NULL 0 +#endif #ifdef __cplusplus extern "C" { -- 2.18.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 32211] [GLSL] lower_jumps with continue-statements in for-loops prevents loop unrolling
https://bugs.freedesktop.org/show_bug.cgi?id=32211 --- Comment #16 from Timothy Arceri --- (In reply to Danylo from comment #15) > Created attachment 142567 [details] [review] > Removing unnecessary continue > > Optimization in question. Thanks! I've tried a few variations of this patch and eventually settled on a v2 which I sent out with an update to loop analysis so that we can detect the duplicate additions. https://patchwork.freedesktop.org/series/53128/ -- 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/4] nir: in loop analysis track actual control flow type
This will allow us to improve analysis to find more induction variables. --- src/compiler/nir/nir_loop_analyze.c | 34 ++--- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/compiler/nir/nir_loop_analyze.c b/src/compiler/nir/nir_loop_analyze.c index 587e9d7865..c804a66ac4 100644 --- a/src/compiler/nir/nir_loop_analyze.c +++ b/src/compiler/nir/nir_loop_analyze.c @@ -49,8 +49,11 @@ typedef struct { /* If this is of type basic_induction */ struct nir_basic_induction_var *ind; - /* True if variable is in an if branch or a nested loop */ - bool in_control_flow; + /* True if variable is in an if branch */ + bool in_if_branch; + + /* True if variable is in a nested loop */ + bool in_nested_loop; } nir_loop_variable; @@ -83,7 +86,8 @@ get_loop_var(nir_ssa_def *value, loop_info_state *state) typedef struct { loop_info_state *state; - bool in_control_flow; + bool in_if_branch; + bool in_nested_loop; } init_loop_state; static bool @@ -92,8 +96,10 @@ init_loop_def(nir_ssa_def *def, void *void_init_loop_state) init_loop_state *loop_init_state = void_init_loop_state; nir_loop_variable *var = get_loop_var(def, loop_init_state->state); - if (loop_init_state->in_control_flow) { - var->in_control_flow = true; + if (loop_init_state->in_nested_loop) { + var->in_nested_loop = true; + } else if (loop_init_state->in_if_branch) { + var->in_if_branch = true; } else { /* Add to the tail of the list. That way we start at the beginning of * the defs in the loop instead of the end when walking the list. This @@ -110,9 +116,10 @@ init_loop_def(nir_ssa_def *def, void *void_init_loop_state) static bool init_loop_block(nir_block *block, loop_info_state *state, -bool in_control_flow) +bool in_if_branch, bool in_nested_loop) { - init_loop_state init_state = {.in_control_flow = in_control_flow, + init_loop_state init_state = {.in_if_branch = in_if_branch, + .in_nested_loop = in_nested_loop, .state = state }; nir_foreach_instr(instr, block) { @@ -198,7 +205,7 @@ compute_invariance_information(loop_info_state *state) */ list_for_each_entry_safe(nir_loop_variable, var, >process_list, process_link) { - assert(!var->in_control_flow); + assert(!var->in_if_branch && !var->in_nested_loop); if (mark_invariant(var->def, state)) list_del(>process_link); @@ -216,7 +223,8 @@ compute_induction_information(loop_info_state *state) * things in nested loops or conditionals should have been removed from * the list by compute_invariance_information(). */ - assert(!var->in_control_flow && var->type != invariant); + assert(!var->in_if_branch && !var->in_nested_loop && + var->type != invariant); /* We are only interested in checking phis for the basic induction * variable case as its simple to detect. All basic induction variables @@ -234,7 +242,7 @@ compute_induction_information(loop_info_state *state) /* If one of the sources is in a conditional or nested block then * panic. */ - if (src_var->in_control_flow) + if (src_var->in_if_branch || src_var->in_nested_loop) break; if (!src_var->in_loop) { @@ -833,17 +841,17 @@ get_loop_info(loop_info_state *state, nir_function_impl *impl) switch (node->type) { case nir_cf_node_block: - init_loop_block(nir_cf_node_as_block(node), state, false); + init_loop_block(nir_cf_node_as_block(node), state, false, false); break; case nir_cf_node_if: nir_foreach_block_in_cf_node(block, node) -init_loop_block(block, state, true); +init_loop_block(block, state, true, false); break; case nir_cf_node_loop: nir_foreach_block_in_cf_node(block, node) { -init_loop_block(block, state, true); +init_loop_block(block, state, false, true); } break; -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/4] nir: reword code comment
--- src/compiler/nir/nir_loop_analyze.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/nir/nir_loop_analyze.c b/src/compiler/nir/nir_loop_analyze.c index c804a66ac4..8903e15105 100644 --- a/src/compiler/nir/nir_loop_analyze.c +++ b/src/compiler/nir/nir_loop_analyze.c @@ -239,8 +239,8 @@ compute_induction_information(loop_info_state *state) nir_foreach_phi_src(src, phi) { nir_loop_variable *src_var = get_loop_var(src->src.ssa, state); - /* If one of the sources is in a conditional or nested block then - * panic. + /* If one of the sources is in an if branch or nested loop then don't + * attempt to go any further. */ if (src_var->in_if_branch || src_var->in_nested_loop) break; -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/4] nir: detect more induction variables
This adds allows loop analysis to detect inductions varibales that are incremented in both branches of an if rather than in a main loop block. For example: loop { block block_1: /* preds: block_0 block_7 */ vec1 32 ssa_8 = phi block_0: ssa_4, block_7: ssa_20 vec1 32 ssa_9 = phi block_0: ssa_0, block_7: ssa_4 vec1 32 ssa_10 = phi block_0: ssa_1, block_7: ssa_4 vec1 32 ssa_11 = phi block_0: ssa_2, block_7: ssa_21 vec1 32 ssa_12 = phi block_0: ssa_3, block_7: ssa_22 vec4 32 ssa_13 = vec4 ssa_12, ssa_11, ssa_10, ssa_9 vec1 32 ssa_14 = ige ssa_8, ssa_5 /* succs: block_2 block_3 */ if ssa_14 { block block_2: /* preds: block_1 */ break /* succs: block_8 */ } else { block block_3: /* preds: block_1 */ /* succs: block_4 */ } block block_4: /* preds: block_3 */ vec1 32 ssa_15 = ilt ssa_6, ssa_8 /* succs: block_5 block_6 */ if ssa_15 { block block_5: /* preds: block_4 */ vec1 32 ssa_16 = iadd ssa_8, ssa_7 vec1 32 ssa_17 = load_const (0x3f80 /* 1.00*/) /* succs: block_7 */ } else { block block_6: /* preds: block_4 */ vec1 32 ssa_18 = iadd ssa_8, ssa_7 vec1 32 ssa_19 = load_const (0x3f80 /* 1.00*/) /* succs: block_7 */ } block block_7: /* preds: block_5 block_6 */ vec1 32 ssa_20 = phi block_5: ssa_16, block_6: ssa_18 vec1 32 ssa_21 = phi block_5: ssa_17, block_6: ssa_4 vec1 32 ssa_22 = phi block_5: ssa_4, block_6: ssa_19 /* succs: block_1 */ } Unfortunatly GCM could move the addition out of the if for us (making this patch unrequired) but we still cannot enable the GCM pass without regressions. This unrolls a loop in Rise of The Tomb Raider. vkpipeline-db results (VEGA): Totals from affected shaders: SGPRS: 88 -> 96 (9.09 %) VGPRS: 56 -> 52 (-7.14 %) Spilled SGPRs: 0 -> 0 (0.00 %) Spilled VGPRs: 0 -> 0 (0.00 %) Private memory VGPRs: 0 -> 0 (0.00 %) Scratch size: 0 -> 0 (0.00 %) dwords per thread Code Size: 2168 -> 4560 (110.33 %) bytes LDS: 0 -> 0 (0.00 %) blocks Max Waves: 4 -> 4 (0.00 %) Wait states: 0 -> 0 (0.00 %) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32211 --- src/compiler/nir/nir_loop_analyze.c | 36 + 1 file changed, 36 insertions(+) diff --git a/src/compiler/nir/nir_loop_analyze.c b/src/compiler/nir/nir_loop_analyze.c index 8903e15105..cf97d6bf06 100644 --- a/src/compiler/nir/nir_loop_analyze.c +++ b/src/compiler/nir/nir_loop_analyze.c @@ -245,6 +245,42 @@ compute_induction_information(loop_info_state *state) if (src_var->in_if_branch || src_var->in_nested_loop) break; + /* Detect inductions varibales that are incremented in both branches + * of an unnested if rather than in a loop block. + */ + if (is_var_phi(src_var)) { +nir_phi_instr *src_phi = + nir_instr_as_phi(src_var->def->parent_instr); + +nir_op alu_op; +nir_ssa_def *alu_srcs[2] = {0}; +nir_foreach_phi_src(src2, src_phi) { + nir_loop_variable *src_var2 = + get_loop_var(src2->src.ssa, state); + + if (!src_var2->in_if_branch || !is_var_alu(src_var2)) + break; + + nir_alu_instr *alu = + nir_instr_as_alu(src_var2->def->parent_instr); + if (nir_op_infos[alu->op].num_inputs != 2) + break; + + if (alu->src[0].src.ssa == alu_srcs[0] && + alu->src[1].src.ssa == alu_srcs[1] && + alu->op == alu_op) { + /* Both branches perform the same calculation so we can use + * one of them to find the induction variable. + */ + src_var = src_var2; + } else { + alu_srcs[0] = alu->src[0].src.ssa; + alu_srcs[1] = alu->src[1].src.ssa; + alu_op = alu->op; + } +} + } + if (!src_var->in_loop) { biv->def_outside_loop = src_var; } else if (is_var_alu(src_var)) { -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by
On 2018-11-27 18:04:17, Matt Turner wrote: > On Tue, Nov 27, 2018 at 6:00 PM Jordan Justen > wrote: > > > > On 2018-11-27 17:17:15, Matt Turner wrote: > > > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen > > > wrote: > > > > > > > > This adds the "Developer's Certificate of Origin 1.1" from the Linux > > > > kernel. It indicates that by using Signed-off-by you are certifying > > > > that your patch meets the DCO 1.1 guidelines. > > > > > > Do we gain anything if it's optional? > > > > As I recall, one thing that bothered you about Signed-off-by in Mesa > > is that it wasn't documented what it meant when it was used. > > > > Perhaps there are developers that don't want to use Signed-off-by with > > an undocumented meaning for Mesa. If that is the case, then this might > > help. I wasn't sure if you fell into that category. > > > > I use -s whenever I commit, so requiring it would not bother me. But, > > I notice that many people (such as yourself) do not, so I didn't see > > the need to push for that. > > > > If it's well documented, and becomes commonly used, then perhaps > > requiring it might be a reasonable thing to consider. I won't be > > holding my breath while waiting on that. :) > > I don't have a problem requiring it. I sign-off on commits I make to > Gentoo, to Linux, etc. If it has the same meaning as with the Linux kernel, but is not required, then you won't use it? I guess your concern might be that you are then giving something to the project that others can choose not to. ? > I'm just against cargo-culting it like we're doing now without a > defined meaning. The purpose of this patch is to give it a defined meaning. And the meaning I chose is the one that people are more likely to have in mind when they use Signed-off-by. Maybe that's too big of an assumption on my part, but I think several other open source projects have followed the kernel on this. > By all means, require it (with a git hook) if you like. I personally don't want to push for that right now. I guess I would like it to be required someday, primarily because it creates a standard process for open source projects to use. (So, people are more likely to be used to it in general when contributing to open source projects.) But, I'm not confident that the consensus for Mesa would be in favor of making that change right now. So, as an alternative I'd like to remove any barriers (such as ambiguity) to its usage in Mesa. -Jordan ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/4] nir: add if opt opt_if_loop_last_continue()
From: Danylo Piliaiev Removing the last continue can allow more loops to unroll. Also inserting code into the if branch can allow the various if opts to progress further. The insertion of some loops into the if branch also reduces VGPR use in some shaders. vkpipeline-db results (VEGA): Totals from affected shaders: SGPRS: 6552 -> 6576 (0.37 %) VGPRS: 6544 -> 6532 (-0.18 %) Spilled SGPRs: 0 -> 0 (0.00 %) Spilled VGPRs: 0 -> 0 (0.00 %) Private memory VGPRs: 0 -> 0 (0.00 %) Scratch size: 0 -> 0 (0.00 %) dwords per thread Code Size: 481952 -> 478032 (-0.81 %) bytes LDS: 13 -> 13 (0.00 %) blocks Max Waves: 241 -> 242 (0.41 %) Wait states: 0 -> 0 (0.00 %) Shader-db results radeonsi (VEGA): Totals from affected shaders: SGPRS: 168 -> 168 (0.00 %) VGPRS: 144 -> 140 (-2.78 %) Spilled SGPRs: 157 -> 157 (0.00 %) Spilled VGPRs: 0 -> 0 (0.00 %) Private memory VGPRs: 0 -> 0 (0.00 %) Scratch size: 0 -> 0 (0.00 %) dwords per thread Code Size: 8524 -> 8488 (-0.42 %) bytes LDS: 0 -> 0 (0.00 %) blocks Max Waves: 7 -> 7 (0.00 %) Wait states: 0 -> 0 (0.00 %) v2: (Timothy Arceri): - allow for continues in either branch - move any trailing loops inside the if as well as blocks. - leave nir_opt_trivial_continues() to actually remove the continue. Signed-off-by: Timothy Arceri Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32211 --- src/compiler/nir/nir_opt_if.c | 95 +++ 1 file changed, 95 insertions(+) diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index dd488b1787..4a9dffb782 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -263,6 +263,100 @@ rewrite_phi_predecessor_blocks(nir_if *nif, } } +static bool +nir_block_ends_in_continue(nir_block *block) +{ + if (exec_list_is_empty(>instr_list)) + return false; + + nir_instr *instr = nir_block_last_instr(block); + return instr->type == nir_instr_type_jump && + nir_instr_as_jump(instr)->type == nir_jump_continue; +} + +/** + * This optimization turns: + * + * loop { + *... + *if (cond) { + * do_work_1(); + * continue; + *} else { + *} + *do_work_2(); + * } + * + * into: + * + * loop { + *... + *if (cond) { + * do_work_1(); + * continue; + *} else { + * do_work_2(); + *} + * } + * + * The continue should then be removed by nir_opt_trivial_continues() and the + * loop can potentially be unrolled. + * + * Note: do_work_2() is only ever blocks and nested loops. We could also nest + * other if-statments in the branch which would allow further continues to + * be removed. However in practice this can result in increased register + * pressure. + */ +static bool +opt_if_loop_last_continue(nir_loop *loop) +{ + /* Get the last if-stament in the loop */ + nir_block *last_block = nir_loop_last_block(loop); + nir_cf_node *if_node = nir_cf_node_prev(_block->cf_node); + while (if_node) { + if (if_node->type == nir_cf_node_if) + break; + + if_node = nir_cf_node_prev(if_node); + } + + if (!if_node || if_node->type != nir_cf_node_if) + return false; + + nir_if *nif = nir_cf_node_as_if(if_node); + nir_block *then_block = nir_if_last_then_block(nif); + nir_block *else_block = nir_if_last_else_block(nif); + + bool then_ends_in_continue = nir_block_ends_in_continue(then_block); + bool else_ends_in_continue = nir_block_ends_in_continue(else_block); + + /* If both branches end in a continue do nothing, this should be handled +* by nir_opt_dead_cf(). +*/ + if (then_ends_in_continue && else_ends_in_continue) + return false; + + if (!then_ends_in_continue && !else_ends_in_continue) + return false; + + /* Move the last block of the loop inside the last if-statement */ + nir_cf_list tmp; + nir_cf_extract(, nir_after_cf_node(if_node), +nir_after_block(last_block)); + if (then_ends_in_continue) { + nir_cf_reinsert(, nir_after_cf_list(>else_list)); + } else { + nir_cf_reinsert(, nir_after_cf_list(>then_list)); + } + + /* In order to avoid running nir_lower_regs_to_ssa_impl() every time an if +* opt makes progress we leave nir_opt_trivial_continues() to remove the +* continue now that the end of the loop has been simplified. +*/ + + return true; +} + /** * This optimization turns: * @@ -700,6 +794,7 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list) nir_loop *loop = nir_cf_node_as_loop(cf_node); progress |= opt_if_cf_list(b, >body); progress |= opt_peel_loop_initial_if(loop); + progress |= opt_if_loop_last_continue(loop); break; } -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process
On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen wrote: > > This documents a mechanism for using GitLab Merge Requests as an > optional, secondary way to get code reviews for patchsets. > > We still require all patches to be emailed. > > Aside from the potential usage for code review comments, it might also > help reviewers to find unmerged patchsets which need review. (Assuming > it doesn't fall into the same fate of patchwork with hundreds of open > MRs.) > > Signed-off-by: Jordan Justen > Cc: Jason Ekstrand > --- > docs/submittingpatches.html | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html > index 5d8ba443191..852f28c198a 100644 > --- a/docs/submittingpatches.html > +++ b/docs/submittingpatches.html > @@ -24,6 +24,7 @@ > Testing Patches > Mailing Patches > Reviewing Patches > +GitLab Merge Requests > Nominating a commit for a stable branch > Criteria for accepting patches to the stable > branch > Sending backports for the stable branch > @@ -282,6 +283,30 @@ which tells the patch author that the patch can be > committed, as long > as the issues are resolved first. > > > +GitLab Merge Requests > + > + > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > + Requests (MR) can be used as an optional, secondary method of > + obtaining code review for patches. > + > + > + > + All patches should be submitted using email as well > + Consider including a link to the MR in your email based > +cover-letter > + Address code review from both email and the MR Discussion point: I think attempting to have simultaneous review in two places is a recipe for wasted time. Strawman: maybe we can only email the cover letter to the mailing list and include in it a link to the MR? > + Add appropriate labels to your MR. For example: > + > + Mesa changes affect all drivers: mesa > + Hardware vendor specific code: amd, intel, nvidia, etc > + Driver specific code: anvil, freedreno, i965, iris, radeonsi, > radv, vc4, etc > + Other tag examples: gallium, util > + > + Never use the merge button on the GitLab page to push patches Can we disable this in Gitlab? If the button is there, people *will* accidentally press it. > + Close your MR when your patches get pushed! Gentoo has some automation that scans the git log for "Closes: ..." and automatically closes the merge requests or bugzilla bugs. Closes: https://github.com/gentoo/gentoo/pull/7423 Closes: https://bugs.gentoo.org/603294 I'm not sure if it's a custom thing or what (I can find out) but I'd much prefer to automate things if we can. Just like patchwork, people *will* forget to close merge requests if it's possible. (And people currently *do* forget to close bugzilla bugs) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] st/mesa: expose GL_OES_texture_view
On Tue, Nov 27, 2018 at 9:00 PM Marek Olšák wrote: > > From: Marek Olšák > > For format fallbacks like ETC and ASTC, switching between sRGB and linear > decoding is undefined, or at least is not bit-exact. Same as > EXT_texture_sRGB_decode on GLES. > > There are no piglit or dEQP regresssions. Just checking -- did you run dEQP on hardware that uses the ASTC fallbacks? If not, could you? Also, what are the conditions for enabling ASTC fallbacks? Basically the issue is that OES_texture_view requires ASTC, so I want to make sure it'll be enabled for any drivers that PIPE_CAP_SAMPLER_VIEW_TARGET is enabled for. Assuming neither of those have surprising responses, this series is Reviewed-by: Ilia Mirkin > --- > docs/features.txt | 2 +- > docs/relnotes/19.0.0.html | 1 + > src/mesa/state_tracker/st_extensions.c | 1 + > 3 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/docs/features.txt b/docs/features.txt > index a97f998a5cc..8999e42519c 100644 > --- a/docs/features.txt > +++ b/docs/features.txt > @@ -331,21 +331,21 @@ Khronos, ARB, and OES extensions that are not part of > any OpenGL or OpenGL ES ve >GL_OES_EGL_image DONE (all drivers) >GL_OES_EGL_image_external DONE (all drivers) >GL_OES_EGL_image_external_essl3 DONE (all drivers) >GL_OES_required_internalformatDONE (all drivers) >GL_OES_surfaceless_contextDONE (all drivers) >GL_OES_texture_compression_astc DONE (core only) >GL_OES_texture_float DONE (freedreno, > i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe) >GL_OES_texture_float_linear DONE (freedreno, > i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe) >GL_OES_texture_half_float DONE (freedreno, > i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe) >GL_OES_texture_half_float_linear DONE (freedreno, > i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe) > - GL_OES_texture_view DONE (i965/gen8+) > + GL_OES_texture_view DONE (i965/gen8+, > freedreno, nv50, r600, radeonsi, nv50, nvc0, llvmpipe, softpipe, swr) >GL_OES_viewport_array DONE (i965, nvc0, > radeonsi) >GLX_ARB_context_flush_control not started >GLX_ARB_robustness_application_isolation not started >GLX_ARB_robustness_share_group_isolation not started > > GL_EXT_direct_state_access subfeatures (in the spec order): >GL 1.1: Client commands not started >GL 1.0-1.3: Matrix and transpose matrix commands not started >GL 1.1-1.2: Texture commands not started >GL 1.2: 3D texture commands not started > diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html > index 1b839b0a485..f66f22132e1 100644 > --- a/docs/relnotes/19.0.0.html > +++ b/docs/relnotes/19.0.0.html > @@ -33,20 +33,21 @@ Compatibility contexts may report a lower version > depending on each driver. > SHA256 checksums > > TBD. > > > > New features > > > GL_EXT_shader_implicit_conversions on all drivers (ES extension). > +GL_OES_texture_view on drivers supporting texture views (ES > extension). > > > Bug fixes > > > TBD > > > Changes > > diff --git a/src/mesa/state_tracker/st_extensions.c > b/src/mesa/state_tracker/st_extensions.c > index 16889074f66..5c068d14e93 100644 > --- a/src/mesa/state_tracker/st_extensions.c > +++ b/src/mesa/state_tracker/st_extensions.c > @@ -760,20 +760,21 @@ void st_init_extensions(struct pipe_screen *screen, >{ o(NV_fill_rectangle), > PIPE_CAP_POLYGON_MODE_FILL_RECTANGLE }, >{ o(NV_primitive_restart), PIPE_CAP_PRIMITIVE_RESTART > }, >{ o(NV_texture_barrier), PIPE_CAP_TEXTURE_BARRIER > }, >{ o(NVX_gpu_memory_info), PIPE_CAP_QUERY_MEMORY_INFO > }, >/* GL_NV_point_sprite is not supported by gallium because we don't > * support the GL_POINT_SPRITE_R_MODE_NV option. */ > >{ o(OES_standard_derivatives), PIPE_CAP_SM3 > }, >{ o(OES_texture_float_linear), PIPE_CAP_TEXTURE_FLOAT_LINEAR > }, >{ o(OES_texture_half_float_linear), > PIPE_CAP_TEXTURE_HALF_FLOAT_LINEAR}, > + { o(OES_texture_view), PIPE_CAP_SAMPLER_VIEW_TARGET > }, > }; > > /* Required: render target and sampler support */ > static const struct st_extension_format_mapping rendertarget_mapping[] = {
Re: [Mesa-dev] [PATCH 6/6] nir: Release per-block metadata in nir_sweep
Rb for this patch On November 27, 2018 18:39:25 "Ian Romanick" wrote: From: Ian Romanick nir_sweep already marks all metadata invalid, so it is safe to release the memory here too. mean soft fp64 using uint64: 1,342,759,331 => 1,010,670,475 gfxbench5 aztec ruins high 11:63,555,571 =>61,889,811 deus ex mankind divided 148: 62,845,304 =>62,829,640 deus ex mankind divided 2890: 71,922,686 =>71,922,686 dirt showdown 676:69,238,607 =>69,238,607 dolphin ubershaders 210: 77,822,072 =>77,822,072 Signed-off-by: Ian Romanick --- src/compiler/nir/nir_sweep.c | 9 + 1 file changed, 9 insertions(+) diff --git a/src/compiler/nir/nir_sweep.c b/src/compiler/nir/nir_sweep.c index aab641388db..b6b56aa078c 100644 --- a/src/compiler/nir/nir_sweep.c +++ b/src/compiler/nir/nir_sweep.c @@ -63,6 +63,15 @@ sweep_block(nir_shader *nir, nir_block *block) { ralloc_steal(nir, block); + /* sweep_impl will mark all metadata invalid. We can safely release all of +* this here. +*/ + ralloc_free(block->live_in); + block->live_in = NULL; + + ralloc_free(block->live_out); + block->live_out = NULL; + nir_foreach_instr(instr, block) { ralloc_steal(nir, instr); -- 2.14.5 ___ 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] docs: Document the optional usage of Signed-off-by
On Tue, Nov 27, 2018 at 6:00 PM Jordan Justen wrote: > > On 2018-11-27 17:17:15, Matt Turner wrote: > > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen > > wrote: > > > > > > This adds the "Developer's Certificate of Origin 1.1" from the Linux > > > kernel. It indicates that by using Signed-off-by you are certifying > > > that your patch meets the DCO 1.1 guidelines. > > > > Do we gain anything if it's optional? > > As I recall, one thing that bothered you about Signed-off-by in Mesa > is that it wasn't documented what it meant when it was used. > > Perhaps there are developers that don't want to use Signed-off-by with > an undocumented meaning for Mesa. If that is the case, then this might > help. I wasn't sure if you fell into that category. > > I use -s whenever I commit, so requiring it would not bother me. But, > I notice that many people (such as yourself) do not, so I didn't see > the need to push for that. > > If it's well documented, and becomes commonly used, then perhaps > requiring it might be a reasonable thing to consider. I won't be > holding my breath while waiting on that. :) I don't have a problem requiring it. I sign-off on commits I make to Gentoo, to Linux, etc. I'm just against cargo-culting it like we're doing now without a defined meaning. By all means, require it (with a git hook) if you like. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] mesa: expose GL_EXT_texture_view as an alias of GL_OES_texture_view
From: Marek Olšák There no spec changes. --- docs/relnotes/19.0.0.html| 1 + src/mapi/glapi/gen/es_EXT.xml| 13 + src/mesa/main/extensions_table.h | 1 + 3 files changed, 15 insertions(+) diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html index f66f22132e1..920cf803f5d 100644 --- a/docs/relnotes/19.0.0.html +++ b/docs/relnotes/19.0.0.html @@ -33,20 +33,21 @@ Compatibility contexts may report a lower version depending on each driver. SHA256 checksums TBD. New features GL_EXT_shader_implicit_conversions on all drivers (ES extension). +GL_EXT_texture_view on drivers supporting texture views (ES extension). GL_OES_texture_view on drivers supporting texture views (ES extension). Bug fixes TBD Changes diff --git a/src/mapi/glapi/gen/es_EXT.xml b/src/mapi/glapi/gen/es_EXT.xml index 459f642e477..bbc4a1a1118 100644 --- a/src/mapi/glapi/gen/es_EXT.xml +++ b/src/mapi/glapi/gen/es_EXT.xml @@ -1453,11 +1453,24 @@ + + + + + + + + + + + + + diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h index dd846d7bc5c..4f2707b65a4 100644 --- a/src/mesa/main/extensions_table.h +++ b/src/mesa/main/extensions_table.h @@ -295,20 +295,21 @@ EXT(EXT_texture_norm16 , dummy_true EXT(EXT_texture_object , dummy_true , GLL, x , x , x , 1995) EXT(EXT_texture_rectangle , NV_texture_rectangle , GLL, x , x , x , 2004) EXT(EXT_texture_rg , ARB_texture_rg , x , x , x , ES2, 2011) EXT(EXT_texture_sRGB, EXT_texture_sRGB , GLL, GLC, x , x , 2004) EXT(EXT_texture_sRGB_R8 , EXT_texture_sRGB_R8 , x , x , x , 30, 2015) EXT(EXT_texture_sRGB_decode , EXT_texture_sRGB_decode , GLL, GLC, x , 30, 2006) EXT(EXT_texture_shared_exponent , EXT_texture_shared_exponent , GLL, GLC, x , x , 2004) EXT(EXT_texture_snorm , EXT_texture_snorm , GLL, GLC, x , x , 2009) EXT(EXT_texture_swizzle , EXT_texture_swizzle , GLL, GLC, x , x , 2008) EXT(EXT_texture_type_2_10_10_10_REV , EXT_texture_type_2_10_10_10_REV , x , x , x , ES2, 2008) +EXT(EXT_texture_view, OES_texture_view , x , x , x , 31, 2014) EXT(EXT_timer_query , EXT_timer_query , GLL, GLC, x , x , 2006) EXT(EXT_transform_feedback , EXT_transform_feedback , GLL, GLC, x , x , 2011) EXT(EXT_unpack_subimage , dummy_true , x , x , x , ES2, 2011) EXT(EXT_vertex_array, dummy_true , GLL, x , x , x , 1995) EXT(EXT_vertex_array_bgra , EXT_vertex_array_bgra , GLL, GLC, x , x , 2008) EXT(EXT_vertex_attrib_64bit , ARB_vertex_attrib_64bit , 32, GLC, x , x , 2010) EXT(EXT_window_rectangles , EXT_window_rectangles , GLL, GLC, x , 30, 2016) EXT(GREMEDY_string_marker , GREMEDY_string_marker , GLL, GLC, x , x , 2007) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] st/mesa: expose GL_OES_texture_view
From: Marek Olšák For format fallbacks like ETC and ASTC, switching between sRGB and linear decoding is undefined, or at least is not bit-exact. Same as EXT_texture_sRGB_decode on GLES. There are no piglit or dEQP regresssions. --- docs/features.txt | 2 +- docs/relnotes/19.0.0.html | 1 + src/mesa/state_tracker/st_extensions.c | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/features.txt b/docs/features.txt index a97f998a5cc..8999e42519c 100644 --- a/docs/features.txt +++ b/docs/features.txt @@ -331,21 +331,21 @@ Khronos, ARB, and OES extensions that are not part of any OpenGL or OpenGL ES ve GL_OES_EGL_image DONE (all drivers) GL_OES_EGL_image_external DONE (all drivers) GL_OES_EGL_image_external_essl3 DONE (all drivers) GL_OES_required_internalformatDONE (all drivers) GL_OES_surfaceless_contextDONE (all drivers) GL_OES_texture_compression_astc DONE (core only) GL_OES_texture_float DONE (freedreno, i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe) GL_OES_texture_float_linear DONE (freedreno, i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe) GL_OES_texture_half_float DONE (freedreno, i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe) GL_OES_texture_half_float_linear DONE (freedreno, i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe) - GL_OES_texture_view DONE (i965/gen8+) + GL_OES_texture_view DONE (i965/gen8+, freedreno, nv50, r600, radeonsi, nv50, nvc0, llvmpipe, softpipe, swr) GL_OES_viewport_array DONE (i965, nvc0, radeonsi) GLX_ARB_context_flush_control not started GLX_ARB_robustness_application_isolation not started GLX_ARB_robustness_share_group_isolation not started GL_EXT_direct_state_access subfeatures (in the spec order): GL 1.1: Client commands not started GL 1.0-1.3: Matrix and transpose matrix commands not started GL 1.1-1.2: Texture commands not started GL 1.2: 3D texture commands not started diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html index 1b839b0a485..f66f22132e1 100644 --- a/docs/relnotes/19.0.0.html +++ b/docs/relnotes/19.0.0.html @@ -33,20 +33,21 @@ Compatibility contexts may report a lower version depending on each driver. SHA256 checksums TBD. New features GL_EXT_shader_implicit_conversions on all drivers (ES extension). +GL_OES_texture_view on drivers supporting texture views (ES extension). Bug fixes TBD Changes diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c index 16889074f66..5c068d14e93 100644 --- a/src/mesa/state_tracker/st_extensions.c +++ b/src/mesa/state_tracker/st_extensions.c @@ -760,20 +760,21 @@ void st_init_extensions(struct pipe_screen *screen, { o(NV_fill_rectangle), PIPE_CAP_POLYGON_MODE_FILL_RECTANGLE }, { o(NV_primitive_restart), PIPE_CAP_PRIMITIVE_RESTART }, { o(NV_texture_barrier), PIPE_CAP_TEXTURE_BARRIER }, { o(NVX_gpu_memory_info), PIPE_CAP_QUERY_MEMORY_INFO }, /* GL_NV_point_sprite is not supported by gallium because we don't * support the GL_POINT_SPRITE_R_MODE_NV option. */ { o(OES_standard_derivatives), PIPE_CAP_SM3 }, { o(OES_texture_float_linear), PIPE_CAP_TEXTURE_FLOAT_LINEAR }, { o(OES_texture_half_float_linear), PIPE_CAP_TEXTURE_HALF_FLOAT_LINEAR}, + { o(OES_texture_view), PIPE_CAP_SAMPLER_VIEW_TARGET }, }; /* Required: render target and sampler support */ static const struct st_extension_format_mapping rendertarget_mapping[] = { { { o(ARB_texture_float) }, { PIPE_FORMAT_R32G32B32A32_FLOAT, PIPE_FORMAT_R16G16B16A16_FLOAT } }, { { o(OES_texture_float) }, { PIPE_FORMAT_R32G32B32A32_FLOAT } }, -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by
On 2018-11-27 17:17:15, Matt Turner wrote: > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen > wrote: > > > > This adds the "Developer's Certificate of Origin 1.1" from the Linux > > kernel. It indicates that by using Signed-off-by you are certifying > > that your patch meets the DCO 1.1 guidelines. > > Do we gain anything if it's optional? As I recall, one thing that bothered you about Signed-off-by in Mesa is that it wasn't documented what it meant when it was used. Perhaps there are developers that don't want to use Signed-off-by with an undocumented meaning for Mesa. If that is the case, then this might help. I wasn't sure if you fell into that category. I use -s whenever I commit, so requiring it would not bother me. But, I notice that many people (such as yourself) do not, so I didn't see the need to push for that. If it's well documented, and becomes commonly used, then perhaps requiring it might be a reasonable thing to consider. I won't be holding my breath while waiting on that. :) -Jordan ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by
On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen wrote: > > This adds the "Developer's Certificate of Origin 1.1" from the Linux > kernel. It indicates that by using Signed-off-by you are certifying > that your patch meets the DCO 1.1 guidelines. Do we gain anything if it's optional? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] docs: Document optional GitLab code review process
This documents a mechanism for using GitLab Merge Requests as an optional, secondary way to get code reviews for patchsets. We still require all patches to be emailed. Aside from the potential usage for code review comments, it might also help reviewers to find unmerged patchsets which need review. (Assuming it doesn't fall into the same fate of patchwork with hundreds of open MRs.) Signed-off-by: Jordan Justen Cc: Jason Ekstrand --- docs/submittingpatches.html | 25 + 1 file changed, 25 insertions(+) diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html index 5d8ba443191..852f28c198a 100644 --- a/docs/submittingpatches.html +++ b/docs/submittingpatches.html @@ -24,6 +24,7 @@ Testing Patches Mailing Patches Reviewing Patches +GitLab Merge Requests Nominating a commit for a stable branch Criteria for accepting patches to the stable branch Sending backports for the stable branch @@ -282,6 +283,30 @@ which tells the patch author that the patch can be committed, as long as the issues are resolved first. +GitLab Merge Requests + + + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge + Requests (MR) can be used as an optional, secondary method of + obtaining code review for patches. + + + + All patches should be submitted using email as well + Consider including a link to the MR in your email based +cover-letter + Address code review from both email and the MR + Add appropriate labels to your MR. For example: + + Mesa changes affect all drivers: mesa + Hardware vendor specific code: amd, intel, nvidia, etc + Driver specific code: anvil, freedreno, i965, iris, radeonsi, radv, vc4, etc + Other tag examples: gallium, util + + Never use the merge button on the GitLab page to push patches + Add Reviewed-by tags to your commits and push using git + Close your MR when your patches get pushed! + Nominating a commit for a stable branch -- 2.20.0.rc1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by
This adds the "Developer's Certificate of Origin 1.1" from the Linux kernel. It indicates that by using Signed-off-by you are certifying that your patch meets the DCO 1.1 guidelines. Signed-off-by: Jordan Justen Cc: Matt Turner --- docs/submittingpatches.html | 53 - 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html index 3f97c941aa5..5d8ba443191 100644 --- a/docs/submittingpatches.html +++ b/docs/submittingpatches.html @@ -20,6 +20,7 @@ Basic guidelines Patch formatting +Patch signing Testing Patches Mailing Patches Reviewing Patches @@ -72,7 +73,9 @@ if needed. For example: is necessary, and removing it causes no regressions in piglit on any platform. -A "Signed-off-by:" line is not required, but not discouraged either. +A "Signed-off-by:" line is not required, but not discouraged +either. The format and meaning of Signed-off-by is documented below in +the patch signing section. If a patch addresses a bugzilla issue, that should be noted in the patch comment. For example: @@ -128,7 +131,55 @@ Please use common sense and do not blindly add everyone. +Patch signing + + Note: Patch signing is optional for the Mesa project. + + + + As described in the patch formatting + section, you can optionally sign your patch by including + Signed-off-by in the patch commit message. The Signed-off-by must + include your real name and email address in this format: + + + Signed-off-by: Joe Hacker jhac...@foo.com + + + By adding Signed-of-by to your contributed patch, you certify that + your contribution meets the guidelines of the Developer's + Certificate of Origin: + + + +Developer's Certificate of Origin 1.1 +- + +By making a contribution to this project, I certify that: + + (a) The contribution was created in whole or in part by me and I + have the right to submit it under the open source license + indicated in the file; or + + (b) The contribution is based upon previous work that, to the best + of my knowledge, is covered under an appropriate open source + license and I have the right under that license to submit that + work with modifications, whether created in whole or in part + by me, under the same open source license (unless I am + permitted to submit under a different license), as indicated + in the file; or + + (c) The contribution was provided directly to me by some other + person who certified (a), (b) or (c) and I have not modified + it. + + (d) I understand and agree that this project and the contribution + are public and that a record of the contribution (including all + personal information I submit with it, including my sign-off) is + maintained indefinitely and may be redistributed consistent with + this project or the open source license(s) involved. + Testing Patches -- 2.20.0.rc1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/6] nir/phi_builder: Use per-value hash table to store [block] -> def mapping
From: Ian Romanick Replace the old array in each value with a hash table in each value. Changes in peak memory usage according to Valgrind massif: mean soft fp64 using uint64: 5,499,875,082 => 1,343,991,403 gfxbench5 aztec ruins high 11:63,619,971 =>63,619,971 deus ex mankind divided 148: 62,887,728 =>62,887,728 deus ex mankind divided 2890: 72,402,222 =>72,399,750 dirt showdown 676:74,466,431 =>69,464,023 dolphin ubershaders 210: 109,630,376 =>78,359,728 Run-time change for a full run on shader-db on my Skylake laptop (with -march=native) is 0.590037% +/- 0.0873431% (n=50). This is about +0.6 seconds on a 111 second run. The previous version of this patch used a single hash table for the whole phi builder. The mapping was from [value, block] -> def, so a separate allocation was needed for each [value, block] tuple. There was quite a bit of per-allocation overhead (due to ralloc), so the patch was followed by a patch that added the use of the slab allocator. The results of those two patches was not quite as good: mean soft fp64 using uint64: 5,499,875,082 => 1,343,991,403 gfxbench5 aztec ruins high 11:63,619,971 =>63,619,971 deus ex mankind divided 148: 62,887,728 =>62,887,728 deus ex mankind divided 2890: 72,402,222 =>72,402,222 * dirt showdown 676:74,466,431 =>72,443,591 * dolphin ubershaders 210: 109,630,376 =>81,034,320 * The * denote tests that are better now. In the tests that are the same in both patches, the "after" peak memory usage was at a different location. I did not check the local peaks. Signed-off-by: Ian Romanick Suggested-by: Jason Ekstrand --- src/compiler/nir/nir_phi_builder.c | 45 ++ 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/src/compiler/nir/nir_phi_builder.c b/src/compiler/nir/nir_phi_builder.c index add3efd2dfc..621777d6ecc 100644 --- a/src/compiler/nir/nir_phi_builder.c +++ b/src/compiler/nir/nir_phi_builder.c @@ -75,9 +75,18 @@ struct nir_phi_builder_value { * - A regular SSA def. This will be either the result of a phi node or *one of the defs provided by nir_phi_builder_value_set_blocK_def(). */ - nir_ssa_def *defs[0]; + struct hash_table ht; }; +/** + * Convert a block index into a value that can be used as a key for a hash table + * + * The hash table functions want a pointer that is not \c NULL. + * _mesa_hash_pointer drops the two least significant bits, but that's where + * most of our data likely is. Shift by 2 and add 1 to make everything happy. + */ +#define INDEX_TO_KEY(x) ((void *)(uintptr_t) ((x << 2) + 1)) + struct nir_phi_builder * nir_phi_builder_create(nir_function_impl *impl) { @@ -111,13 +120,16 @@ nir_phi_builder_add_value(struct nir_phi_builder *pb, unsigned num_components, struct nir_phi_builder_value *val; unsigned i, w_start = 0, w_end = 0; - val = rzalloc_size(pb, sizeof(*val) + sizeof(val->defs[0]) * pb->num_blocks); + val = rzalloc_size(pb, sizeof(*val)); val->builder = pb; val->num_components = num_components; val->bit_size = bit_size; exec_list_make_empty(>phis); exec_list_push_tail(>values, >node); + _mesa_hash_table_init(>ht, pb, _mesa_hash_pointer, + _mesa_key_pointer_equal); + pb->iter_count++; BITSET_WORD tmp; @@ -142,7 +154,7 @@ nir_phi_builder_add_value(struct nir_phi_builder *pb, unsigned num_components, if (next == pb->impl->end_block) continue; - if (val->defs[next->index] == NULL) { + if (_mesa_hash_table_search(>ht, INDEX_TO_KEY(next->index)) == NULL) { /* Instead of creating a phi node immediately, we simply set the * value to the magic value NEEDS_PHI. Later, we create phi nodes * on demand in nir_phi_builder_value_get_block_def(). @@ -164,7 +176,7 @@ void nir_phi_builder_value_set_block_def(struct nir_phi_builder_value *val, nir_block *block, nir_ssa_def *def) { - val->defs[block->index] = def; + _mesa_hash_table_insert(>ht, INDEX_TO_KEY(block->index), def); } nir_ssa_def * @@ -175,8 +187,18 @@ nir_phi_builder_value_get_block_def(struct nir_phi_builder_value *val, * have a valid ssa_def, if any. */ nir_block *dom = block; - while (dom && val->defs[dom->index] == NULL) + struct hash_entry *he = NULL; + + while (dom != NULL) { + he = _mesa_hash_table_search(>ht, INDEX_TO_KEY(dom->index)); + if (he != NULL) + break; + dom = dom->imm_dom; + } + + /* Exactly one of (he != NULL) and (dom == NULL) must be true. */ + assert((he != NULL) != (dom == NULL)); nir_ssa_def *def; if (dom == NULL) { @@ -191,7 +213,7 @@ nir_phi_builder_value_get_block_def(struct nir_phi_builder_value *val, nir_instr_insert(nir_before_cf_list(>builder->impl->body),
[Mesa-dev] [PATCH 2/6] util/slab: Rename slab_mempool typed parameters to mempool
From: Ian Romanick Now everything with type 'struct slab_child_pool *' is name pool, and everything with type 'struct slab_mempool *' is named mempool. Signed-off-by: Ian Romanick --- src/util/slab.c | 20 ++-- src/util/slab.h | 8 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/util/slab.c b/src/util/slab.c index 5f048666b56..5477c75d443 100644 --- a/src/util/slab.c +++ b/src/util/slab.c @@ -280,25 +280,25 @@ void slab_free(struct slab_child_pool *pool, void *ptr) * Allocate an object from the slab. Single-threaded (no mutex). */ void * -slab_alloc_st(struct slab_mempool *pool) +slab_alloc_st(struct slab_mempool *mempool) { - return slab_alloc(>child); + return slab_alloc(>child); } /** * Free an object allocated from the slab. Single-threaded (no mutex). */ void -slab_free_st(struct slab_mempool *pool, void *ptr) +slab_free_st(struct slab_mempool *mempool, void *ptr) { - slab_free(>child, ptr); + slab_free(>child, ptr); } void -slab_destroy(struct slab_mempool *pool) +slab_destroy(struct slab_mempool *mempool) { - slab_destroy_child(>child); - slab_destroy_parent(>parent); + slab_destroy_child(>child); + slab_destroy_parent(>parent); } /** @@ -308,10 +308,10 @@ slab_destroy(struct slab_mempool *pool) * \param num_items Number of objects to allocate at once. */ void -slab_create(struct slab_mempool *pool, +slab_create(struct slab_mempool *mempool, unsigned item_size, unsigned num_items) { - slab_create_parent(>parent, item_size, num_items); - slab_create_child(>child, >parent); + slab_create_parent(>parent, item_size, num_items); + slab_create_child(>child, >parent); } diff --git a/src/util/slab.h b/src/util/slab.h index e83f8ec1a0e..5a25adaf7f4 100644 --- a/src/util/slab.h +++ b/src/util/slab.h @@ -84,11 +84,11 @@ struct slab_mempool { struct slab_child_pool child; }; -void slab_create(struct slab_mempool *pool, +void slab_create(struct slab_mempool *mempool, unsigned item_size, unsigned num_items); -void slab_destroy(struct slab_mempool *pool); -void *slab_alloc_st(struct slab_mempool *pool); -void slab_free_st(struct slab_mempool *pool, void *ptr); +void slab_destroy(struct slab_mempool *mempool); +void *slab_alloc_st(struct slab_mempool *mempool); +void slab_free_st(struct slab_mempool *mempool, void *ptr); #endif -- 2.14.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/6] util/hash_table: Add _mesa_hash_table_init function
From: Ian Romanick Signed-off-by: Ian Romanick --- src/util/hash_table.c | 39 ++- src/util/hash_table.h | 8 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/util/hash_table.c b/src/util/hash_table.c index fc152f84a4d..4f510612a8f 100644 --- a/src/util/hash_table.c +++ b/src/util/hash_table.c @@ -110,6 +110,27 @@ entry_is_present(const struct hash_table *ht, struct hash_entry *entry) return entry->key != NULL && entry->key != ht->deleted_key; } +bool +_mesa_hash_table_init(struct hash_table *ht, + void *mem_ctx, + uint32_t (*key_hash_function)(const void *key), + bool (*key_equals_function)(const void *a, + const void *b)) +{ + ht->size_index = 0; + ht->size = hash_sizes[ht->size_index].size; + ht->rehash = hash_sizes[ht->size_index].rehash; + ht->max_entries = hash_sizes[ht->size_index].max_entries; + ht->key_hash_function = key_hash_function; + ht->key_equals_function = key_equals_function; + ht->table = rzalloc_array(mem_ctx, struct hash_entry, ht->size); + ht->entries = 0; + ht->deleted_entries = 0; + ht->deleted_key = _key_value; + + return ht->table != NULL; +} + struct hash_table * _mesa_hash_table_create(void *mem_ctx, uint32_t (*key_hash_function)(const void *key), @@ -118,22 +139,14 @@ _mesa_hash_table_create(void *mem_ctx, { struct hash_table *ht; + /* mem_ctx is used to allocate the hash table, but the hash table is used +* to allocate all of the suballocations. +*/ ht = ralloc(mem_ctx, struct hash_table); if (ht == NULL) return NULL; - ht->size_index = 0; - ht->size = hash_sizes[ht->size_index].size; - ht->rehash = hash_sizes[ht->size_index].rehash; - ht->max_entries = hash_sizes[ht->size_index].max_entries; - ht->key_hash_function = key_hash_function; - ht->key_equals_function = key_equals_function; - ht->table = rzalloc_array(ht, struct hash_entry, ht->size); - ht->entries = 0; - ht->deleted_entries = 0; - ht->deleted_key = _key_value; - - if (ht->table == NULL) { + if (!_mesa_hash_table_init(ht, ht, key_hash_function, key_equals_function)) { ralloc_free(ht); return NULL; } @@ -287,7 +300,7 @@ _mesa_hash_table_rehash(struct hash_table *ht, unsigned new_size_index) if (new_size_index >= ARRAY_SIZE(hash_sizes)) return; - table = rzalloc_array(ht, struct hash_entry, + table = rzalloc_array(ralloc_parent(ht->table), struct hash_entry, hash_sizes[new_size_index].size); if (table == NULL) return; diff --git a/src/util/hash_table.h b/src/util/hash_table.h index d89fc1dc1c8..40acda1fd1e 100644 --- a/src/util/hash_table.h +++ b/src/util/hash_table.h @@ -62,6 +62,14 @@ _mesa_hash_table_create(void *mem_ctx, uint32_t (*key_hash_function)(const void *key), bool (*key_equals_function)(const void *a, const void *b)); + +bool +_mesa_hash_table_init(struct hash_table *ht, + void *mem_ctx, + uint32_t (*key_hash_function)(const void *key), + bool (*key_equals_function)(const void *a, + const void *b)); + struct hash_table * _mesa_hash_table_clone(struct hash_table *src, void *dst_mem_ctx); void _mesa_hash_table_destroy(struct hash_table *ht, -- 2.14.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 5/6] nir: Fix holes in nir_instr
From: Ian Romanick Found using pahole. Changes in peak memory usage according to Valgrind massif: mean soft fp64 using uint64: 1,343,991,403 => 1,342,759,331 gfxbench5 aztec ruins high 11:63,619,971 =>63,555,571 deus ex mankind divided 148: 62,887,728 =>62,845,304 deus ex mankind divided 2890: 72,399,750 =>71,922,686 dirt showdown 676:69,464,023 =>69,238,607 dolphin ubershaders 210: 78,359,728 =>77,822,072 Signed-off-by: Ian Romanick --- src/compiler/nir/nir.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index a292ec73e1e..74c700026ad 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -486,7 +486,7 @@ typedef struct nir_register { #define nir_foreach_register_safe(reg, reg_list) \ foreach_list_typed_safe(nir_register, reg, node, reg_list) -typedef enum { +typedef enum PACKED { nir_instr_type_alu, nir_instr_type_deref, nir_instr_type_call, @@ -501,16 +501,16 @@ typedef enum { typedef struct nir_instr { struct exec_node node; - nir_instr_type type; struct nir_block *block; - - /** generic instruction index. */ - unsigned index; + nir_instr_type type; /* A temporary for optimization and analysis passes to use for storing * flags. For instance, DCE uses this to store the "dead/live" info. */ uint8_t pass_flags; + + /** generic instruction index. */ + unsigned index; } nir_instr; static inline nir_instr * -- 2.14.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/6] nir/phi_builder: Internal users should use nir_phi_builder_value_set_block_def too
From: Ian Romanick Signed-off-by: Ian Romanick Reviewed-by: Jason Ekstrand --- src/compiler/nir/nir_phi_builder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/nir/nir_phi_builder.c b/src/compiler/nir/nir_phi_builder.c index cc5ce81d120..add3efd2dfc 100644 --- a/src/compiler/nir/nir_phi_builder.c +++ b/src/compiler/nir/nir_phi_builder.c @@ -147,7 +147,7 @@ nir_phi_builder_add_value(struct nir_phi_builder *pb, unsigned num_components, * value to the magic value NEEDS_PHI. Later, we create phi nodes * on demand in nir_phi_builder_value_get_block_def(). */ -val->defs[next->index] = NEEDS_PHI; +nir_phi_builder_value_set_block_def(val, next, NEEDS_PHI); if (pb->work[next->index] < pb->iter_count) { pb->work[next->index] = pb->iter_count; @@ -232,7 +232,7 @@ nir_phi_builder_value_get_block_def(struct nir_phi_builder_value *val, * 2) To avoid unneeded recreation of phi nodes and undefs. */ for (dom = block; dom && val->defs[dom->index] == NULL; dom = dom->imm_dom) - val->defs[dom->index] = def; + nir_phi_builder_value_set_block_def(val, dom, def); return def; } -- 2.14.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 0/6]
After incorporating several of Jason's suggestions, I have decided to just resend this series. The end result is the same for the worst case. The other cases are 1% - 4% less peak memory usage. The new series is also about 80 lines less code (git diff phi-builder-v1..phi-builder-v2 is 72 insertions(+), 151 deletions(-)). The new series is: https://cgit.freedesktop.org/~idr/mesa/log/?h=phi-builder-v2 The old series (with trivial changes) is: https://cgit.freedesktop.org/~idr/mesa/log/?h=phi-builder-v1 I just re-ordered the old series so that the first two patches of both are the same. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/6] nir: Release per-block metadata in nir_sweep
From: Ian Romanick nir_sweep already marks all metadata invalid, so it is safe to release the memory here too. mean soft fp64 using uint64: 1,342,759,331 => 1,010,670,475 gfxbench5 aztec ruins high 11:63,555,571 =>61,889,811 deus ex mankind divided 148: 62,845,304 =>62,829,640 deus ex mankind divided 2890: 71,922,686 =>71,922,686 dirt showdown 676:69,238,607 =>69,238,607 dolphin ubershaders 210: 77,822,072 =>77,822,072 Signed-off-by: Ian Romanick --- src/compiler/nir/nir_sweep.c | 9 + 1 file changed, 9 insertions(+) diff --git a/src/compiler/nir/nir_sweep.c b/src/compiler/nir/nir_sweep.c index aab641388db..b6b56aa078c 100644 --- a/src/compiler/nir/nir_sweep.c +++ b/src/compiler/nir/nir_sweep.c @@ -63,6 +63,15 @@ sweep_block(nir_shader *nir, nir_block *block) { ralloc_steal(nir, block); + /* sweep_impl will mark all metadata invalid. We can safely release all of +* this here. +*/ + ralloc_free(block->live_in); + block->live_in = NULL; + + ralloc_free(block->live_out); + block->live_out = NULL; + nir_foreach_instr(instr, block) { ralloc_steal(nir, instr); -- 2.14.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Use nir's info when checking uses_streams.
Vulkan and Gallium don't use Mesa's gl_program data structure, so they can't poke at 'prog'. But we can simply use the copy of the shader info stored with the NIR shader, which is guaranteed to exist. Reviewed-by: Jason Ekstrand --- src/intel/compiler/brw_vec4_gs_visitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_vec4_gs_visitor.cpp b/src/intel/compiler/brw_vec4_gs_visitor.cpp index 63ff27e5e08..a6e38b0f379 100644 --- a/src/intel/compiler/brw_vec4_gs_visitor.cpp +++ b/src/intel/compiler/brw_vec4_gs_visitor.cpp @@ -667,7 +667,7 @@ brw_compile_gs(const struct brw_compiler *compiler, void *log_data, prog_data->control_data_format = GEN7_GS_CONTROL_DATA_FORMAT_GSCTL_SID; /* We only have to emit control bits if we are using streams */ - if (prog && prog->info.gs.uses_streams) + if (shader->info.gs.uses_streams) c.control_data_bits_per_vertex = 2; else c.control_data_bits_per_vertex = 0; -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Please bring back __GL_FSAA_MODE
Yes, it could be added back. Marek On Tue, Nov 27, 2018 at 3:15 AM Tom Butler wrote: > Hello, > > I realise this was removed for a reason ( > https://lists.freedesktop.org/archives/mesa-dev/2014-September/067864.html > ) but there are cases where it is useful. In older games that do not > support native FSAA being able to force it in the driver is the only way > to enable it. > > > A quick google search for AMD linux force msaa shows that I'm not the > only one who would like to see this feature return: > > > https://www.phoronix.com/forums/forum/linux-graphics-x-org-drivers/open-source-amd-linux/1024166-radeon-eqaa-anti-aliasing-support-merged-to-mesa-18-2?p=1024185#post1024185 > > https://bbs.archlinux.org/viewtopic.php?id=225425 > > > https://www.reddit.com/r/linux_gaming/comments/671yzm/forcing_antialiasing_with_mesa_radeon_driver/ > > https://github.com/dscharrer/void/blob/master/hacks/forcemsaa.c > > I understand it does cause issues in some cases but there are times when > it is useful. Could it be reintroduced with a more relevant name that > implies it shouldn't be used? E.g. GALLIUM_LEGACY_MSAA or > GALLIUM_FORCE_MSAA_EXPERIMENTAL. That way it would lower users > expectations while still making the option available to try. > > > Kind Regards, > > Tom Butler > > > > ___ > 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] gallium: Remove unused variable in u_tests.
Reviewed-by: Marek Olšák Marek On Tue, Nov 27, 2018 at 2:26 PM Eric Anholt wrote: > Fixes: 0d17b685b1ff ("gallium/u_tests: add a compute shader test that > clears an image") > --- > src/gallium/auxiliary/util/u_tests.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/src/gallium/auxiliary/util/u_tests.c > b/src/gallium/auxiliary/util/u_tests.c > index 7c4e98402bef..365d4fa8f171 100644 > --- a/src/gallium/auxiliary/util/u_tests.c > +++ b/src/gallium/auxiliary/util/u_tests.c > @@ -792,7 +792,6 @@ test_compute_clear_image(struct pipe_context *ctx) > { > struct cso_context *cso; > struct pipe_resource *cb; > - struct pipe_sampler_view *view = NULL; > const char *text; > > cso = cso_create_context(ctx, 0); > -- > 2.20.0.rc1 > > ___ > 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] virgl: Don't try handling server fences when they are not supported
This patch has been: Reviewed-by: Robert Foss On 2018-11-27 20:50, Gert Wollny wrote: From: Gert Wollny vtest doesn't implement the according API and would segfault: Program received signal SIGSEGV, Segmentation fault. #0 0x in ?? () #1 in virgl_fence_server_sync at src/gallium/drivers/virgl/virgl_context.c:1049 #2 in st_server_wait_sync at src/mesa/state_tracker/st_cb_syncobj.c:155 so just don't do the call when the function pointers are not set. Fixes dEQP: dEQP-GLES3.functional.fence_sync.wait_sync_smalldraw dEQP-GLES3.functional.fence_sync.wait_sync_largedraw Fixes: d1a1c21e7621b5177febf191fcd3d3b8ef69dc96 virgl: native fence fd support Signed-off-by: Gert Wollny --- src/gallium/drivers/virgl/virgl_context.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/virgl/virgl_context.c b/src/gallium/drivers/virgl/virgl_context.c index 892fef76c7..f0ee64c145 100644 --- a/src/gallium/drivers/virgl/virgl_context.c +++ b/src/gallium/drivers/virgl/virgl_context.c @@ -1037,7 +1037,8 @@ static void virgl_create_fence_fd(struct pipe_context *ctx, assert(type == PIPE_FD_TYPE_NATIVE_SYNC); struct virgl_screen *rs = virgl_screen(ctx->screen); - *fence = rs->vws->cs_create_fence(rs->vws, fd); + if (rs->vws->cs_create_fence) + *fence = rs->vws->cs_create_fence(rs->vws, fd); } static void virgl_fence_server_sync(struct pipe_context *ctx, @@ -1046,7 +1047,8 @@ static void virgl_fence_server_sync(struct pipe_context *ctx, struct virgl_context *vctx = virgl_context(ctx); struct virgl_screen *rs = virgl_screen(ctx->screen); - rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); + if (rs->vws->fence_server_sync) + rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); } static void virgl_set_shader_images(struct pipe_context *ctx, ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] virgl,vtest: Initialize return value
This patch has been: Reviewed-by: Robert Foss On 2018-11-27 20:50, Gert Wollny wrote: From: Gert Wollny Avoids: Conditional jump or move depends on uninitialised value(s) at 0x9E2B39F: virgl_vtest_winsys_resource_cache_create (virgl_vtest_winsys.c:379) by 0x9E2725F: virgl_buffer_create (virgl_buffer.c:169) by 0x9E246D5: virgl_resource_create (virgl_resource.c:60) by 0xA0C1B9F: bufferobj_data (st_cb_bufferobjects.c:344) by 0xA0C1B9F: st_bufferobj_data (st_cb_bufferobjects.c:390) by 0x9F4ACE3: vbo_use_buffer_objects (vbo_exec_api.c:1136) by 0xA0C68C3: st_create_context_priv (st_context.c:416) by 0xA0C707A: st_create_context (st_context.c:598) by 0x9F81C6B: st_api_create_context (st_manager.c:918) by 0x9BBE591: dri_create_context (dri_context.c:161) by 0x9BB6931: driCreateContextAttribs (dri_util.c:473) by 0x4E97A44: drisw_create_context_attribs (drisw_glx.c:630) by 0x4E7C591: glXCreateContextAttribsARB (create_context.c:78) Uninitialised value was created by a stack allocation at 0x9E2B249: virgl_vtest_winsys_resource_cache_create (virgl_vtest_winsys.c:342) Signed-off-by: Gert Wollny --- src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c index a23f853924..65963ad50e 100644 --- a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c +++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c @@ -344,7 +344,7 @@ virgl_vtest_winsys_resource_cache_create(struct virgl_winsys *vws, struct virgl_hw_res *res, *curr_res; struct list_head *curr, *next; int64_t now; - int ret; + int ret = -1; /* only store binds for vertex/index/const buffers */ if (bind != VIRGL_BIND_CONSTANT_BUFFER && bind != VIRGL_BIND_INDEX_BUFFER && ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] wsi/display: fix mem leak when freeing swapchains
Eric Engestrom writes: > Fixes: da997ebec92942193955 "vulkan: Add KHR_display extension using DRM > [v10]" > Cc: Keith Packard > Signed-off-by: Eric Engestrom Reviewed-by: Keith Packard I checked to ensure that this is sufficient to clean all allocated objects: * Creating a display swapchain allocates the chain, image_count images and call wsi_swapchain_init. Adding the wsi_swapchain_finish cleans the last of these allocations. * The queue of to-be-presented images exists only as state values within the images themselves, so there's no additional allocation there. -- -keith signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] virgl: Don't try handling server fences when they are not supported
On Wed, 28 Nov 2018 at 05:51, Gert Wollny wrote: > > From: Gert Wollny > > vtest doesn't implement the according API and would segfault: > > Program received signal SIGSEGV, Segmentation fault. > #0 0x in ?? () > #1 in virgl_fence_server_sync at >src/gallium/drivers/virgl/virgl_context.c:1049 > #2 in st_server_wait_sync at >src/mesa/state_tracker/st_cb_syncobj.c:155 > > so just don't do the call when the function pointers are not set. > > Fixes dEQP: > dEQP-GLES3.functional.fence_sync.wait_sync_smalldraw > dEQP-GLES3.functional.fence_sync.wait_sync_largedraw > > Fixes: d1a1c21e7621b5177febf191fcd3d3b8ef69dc96 > virgl: native fence fd support > > Signed-off-by: Gert Wollny Reviewed-by: Dave Airlie For both. Dave. > --- > src/gallium/drivers/virgl/virgl_context.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/src/gallium/drivers/virgl/virgl_context.c > b/src/gallium/drivers/virgl/virgl_context.c > index 892fef76c7..f0ee64c145 100644 > --- a/src/gallium/drivers/virgl/virgl_context.c > +++ b/src/gallium/drivers/virgl/virgl_context.c > @@ -1037,7 +1037,8 @@ static void virgl_create_fence_fd(struct pipe_context > *ctx, > assert(type == PIPE_FD_TYPE_NATIVE_SYNC); > struct virgl_screen *rs = virgl_screen(ctx->screen); > > - *fence = rs->vws->cs_create_fence(rs->vws, fd); > + if (rs->vws->cs_create_fence) > + *fence = rs->vws->cs_create_fence(rs->vws, fd); > } > > static void virgl_fence_server_sync(struct pipe_context *ctx, > @@ -1046,7 +1047,8 @@ static void virgl_fence_server_sync(struct pipe_context > *ctx, > struct virgl_context *vctx = virgl_context(ctx); > struct virgl_screen *rs = virgl_screen(ctx->screen); > > - rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); > + if (rs->vws->fence_server_sync) > + rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); > } > > static void virgl_set_shader_images(struct pipe_context *ctx, > -- > 2.18.1 > > ___ > 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] virgl: Don't try handling server fences when they are not supported
From: Gert Wollny vtest doesn't implement the according API and would segfault: Program received signal SIGSEGV, Segmentation fault. #0 0x in ?? () #1 in virgl_fence_server_sync at src/gallium/drivers/virgl/virgl_context.c:1049 #2 in st_server_wait_sync at src/mesa/state_tracker/st_cb_syncobj.c:155 so just don't do the call when the function pointers are not set. Fixes dEQP: dEQP-GLES3.functional.fence_sync.wait_sync_smalldraw dEQP-GLES3.functional.fence_sync.wait_sync_largedraw Fixes: d1a1c21e7621b5177febf191fcd3d3b8ef69dc96 virgl: native fence fd support Signed-off-by: Gert Wollny --- src/gallium/drivers/virgl/virgl_context.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/virgl/virgl_context.c b/src/gallium/drivers/virgl/virgl_context.c index 892fef76c7..f0ee64c145 100644 --- a/src/gallium/drivers/virgl/virgl_context.c +++ b/src/gallium/drivers/virgl/virgl_context.c @@ -1037,7 +1037,8 @@ static void virgl_create_fence_fd(struct pipe_context *ctx, assert(type == PIPE_FD_TYPE_NATIVE_SYNC); struct virgl_screen *rs = virgl_screen(ctx->screen); - *fence = rs->vws->cs_create_fence(rs->vws, fd); + if (rs->vws->cs_create_fence) + *fence = rs->vws->cs_create_fence(rs->vws, fd); } static void virgl_fence_server_sync(struct pipe_context *ctx, @@ -1046,7 +1047,8 @@ static void virgl_fence_server_sync(struct pipe_context *ctx, struct virgl_context *vctx = virgl_context(ctx); struct virgl_screen *rs = virgl_screen(ctx->screen); - rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); + if (rs->vws->fence_server_sync) + rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); } static void virgl_set_shader_images(struct pipe_context *ctx, -- 2.18.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] virgl,vtest: Initialize return value
From: Gert Wollny Avoids: Conditional jump or move depends on uninitialised value(s) at 0x9E2B39F: virgl_vtest_winsys_resource_cache_create (virgl_vtest_winsys.c:379) by 0x9E2725F: virgl_buffer_create (virgl_buffer.c:169) by 0x9E246D5: virgl_resource_create (virgl_resource.c:60) by 0xA0C1B9F: bufferobj_data (st_cb_bufferobjects.c:344) by 0xA0C1B9F: st_bufferobj_data (st_cb_bufferobjects.c:390) by 0x9F4ACE3: vbo_use_buffer_objects (vbo_exec_api.c:1136) by 0xA0C68C3: st_create_context_priv (st_context.c:416) by 0xA0C707A: st_create_context (st_context.c:598) by 0x9F81C6B: st_api_create_context (st_manager.c:918) by 0x9BBE591: dri_create_context (dri_context.c:161) by 0x9BB6931: driCreateContextAttribs (dri_util.c:473) by 0x4E97A44: drisw_create_context_attribs (drisw_glx.c:630) by 0x4E7C591: glXCreateContextAttribsARB (create_context.c:78) Uninitialised value was created by a stack allocation at 0x9E2B249: virgl_vtest_winsys_resource_cache_create (virgl_vtest_winsys.c:342) Signed-off-by: Gert Wollny --- src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c index a23f853924..65963ad50e 100644 --- a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c +++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c @@ -344,7 +344,7 @@ virgl_vtest_winsys_resource_cache_create(struct virgl_winsys *vws, struct virgl_hw_res *res, *curr_res; struct list_head *curr, *next; int64_t now; - int ret; + int ret = -1; /* only store binds for vertex/index/const buffers */ if (bind != VIRGL_BIND_CONSTANT_BUFFER && bind != VIRGL_BIND_INDEX_BUFFER && -- 2.18.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 4/5] intel/compiler: Implement unordered comparisons
On Tue, Nov 27, 2018 at 12:55 PM Ian Romanick wrote: > On 11/22/2018 10:46 AM, Jason Ekstrand wrote: > > The vec4 path has only been compile-tested as there's no easy way to > > generate a vec4 shader with an unordered equality. > > --- > > src/intel/compiler/brw_compiler.c | 3 +++ > > src/intel/compiler/brw_fs_nir.cpp | 20 +--- > > src/intel/compiler/brw_vec4_nir.cpp | 21 + > > 3 files changed, 37 insertions(+), 7 deletions(-) > > > > diff --git a/src/intel/compiler/brw_compiler.c > b/src/intel/compiler/brw_compiler.c > > index fe632c5badc..f9e8fa09a34 100644 > > --- a/src/intel/compiler/brw_compiler.c > > +++ b/src/intel/compiler/brw_compiler.c > > @@ -42,6 +42,9 @@ > > .lower_fdiv = true, > \ > > .lower_flrp64 = true, > \ > > .lower_ldexp = true, >\ > > + .lower_fltu = true, > \ > > + .lower_fgeu = true, > \ > > Can't we use cmpn.l and cmpn.ge for these? On some platforms it has to > be split into two SIMD8 instructions, but that seems better than the > lowering... or maybe just use those on BDW+? > I didn't know about CMPN! I guess that would do the trick though I suppose we have a pile of back-end work to be able to optimize it. I think the back-end usually gets rid of the inot by flipping the cmod but maybe not. > > + .lower_fne_to_fequ = true, >\ > > .lower_cs_local_id_from_index = true, > \ > > .lower_device_index_to_zero = true, > \ > > .native_integers = true, >\ > > diff --git a/src/intel/compiler/brw_fs_nir.cpp > b/src/intel/compiler/brw_fs_nir.cpp > > index a62d521bb5d..eba3611e447 100644 > > --- a/src/intel/compiler/brw_fs_nir.cpp > > +++ b/src/intel/compiler/brw_fs_nir.cpp > > @@ -1049,6 +1049,8 @@ fs_visitor::nir_emit_alu(const fs_builder , > nir_alu_instr *instr) > > case nir_op_flt: > > case nir_op_fge: > > case nir_op_feq: > > + case nir_op_fne: > > + case nir_op_fequ: > > case nir_op_fneu: { > >fs_reg dest = result; > > > > @@ -1056,26 +1058,30 @@ fs_visitor::nir_emit_alu(const fs_builder , > nir_alu_instr *instr) > >if (bit_size != 32) > > dest = bld.vgrf(op[0].type, 1); > > > > - brw_conditional_mod cond; > >switch (instr->op) { > >case nir_op_flt: > > - cond = BRW_CONDITIONAL_L; > > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_L); > > break; > >case nir_op_fge: > > - cond = BRW_CONDITIONAL_GE; > > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_GE); > > break; > >case nir_op_feq: > > - cond = BRW_CONDITIONAL_Z; > > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_Z); > > + break; > > + case nir_op_fequ: > > + bld.CMP(dest, op[0], op[0], BRW_CONDITIONAL_NZ); > > + set_predicate_inv(BRW_PREDICATE_NORMAL, true, /* inverse */ > > + bld.CMP(dest, op[1], op[1], > BRW_CONDITIONAL_NZ)); > > + set_predicate_inv(BRW_PREDICATE_NORMAL, true, /* inverse */ > > + bld.CMP(dest, op[0], op[1], > BRW_CONDITIONAL_Z)); > > OpFUnordEqual is (isnan(a) || isnan(b) || a == b), and that's literally > what this code does. It seems like this could be implemented (perhaps > as a lowering?) as (OpFUnordGreaterThanEqual(a,b) && > OpFUnordGreaterThanEqual(b, a)) via cmpn.ge. Using this same technique, > that would be 2 instructions (on BDW+) instead of 3. HSW has to do cmpn > as two SIMD8 instructions, so this may be better there. Dunno. > That's a good idea. Unfortunately, CMPN dosn't modify Z or NZ. We could also lower fequ(x, y) to inot(fne(x, y)) and implement fne(x, y) as (x < y) || (x > y) which wouldn't require CMPN. --Jason > > break; > >case nir_op_fneu: > > - cond = BRW_CONDITIONAL_NZ; > > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_NZ); > > break; > >default: > > unreachable("bad opcode"); > >} > > > > - bld.CMP(dest, op[0], op[1], cond); > > - > >if (bit_size > 32) { > > bld.MOV(result, subscript(dest, BRW_REGISTER_TYPE_UD, 0)); > >} else if(bit_size < 32) { > > diff --git a/src/intel/compiler/brw_vec4_nir.cpp > b/src/intel/compiler/brw_vec4_nir.cpp > > index f7f46f5034c..32559e1aade 100644 > > --- a/src/intel/compiler/brw_vec4_nir.cpp > > +++ b/src/intel/compiler/brw_vec4_nir.cpp > > @@ -1366,6 +1366,27 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr) > >break; > > } > > > > + case nir_op_fequ: { > > + dst_reg cmp_res = dst; > > + if (nir_src_bit_size(instr->src[0].src) == 64) > > + cmp_res = dst_reg(this, glsl_type::dvec4_type); > > + > > + vec4_instruction *inst; > > + inst = emit(CMP(cmp_res, op[0], op[0], BRW_CONDITIONAL_NZ)); > > + inst = emit(CMP(cmp_res, op[1], op[1], BRW_CONDITIONAL_NZ)); > > + inst->predicate = BRW_PREDICATE_NORMAL; > > +
[Mesa-dev] [PATCH] gallium: Remove unused variable in u_tests.
Fixes: 0d17b685b1ff ("gallium/u_tests: add a compute shader test that clears an image") --- src/gallium/auxiliary/util/u_tests.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/gallium/auxiliary/util/u_tests.c b/src/gallium/auxiliary/util/u_tests.c index 7c4e98402bef..365d4fa8f171 100644 --- a/src/gallium/auxiliary/util/u_tests.c +++ b/src/gallium/auxiliary/util/u_tests.c @@ -792,7 +792,6 @@ test_compute_clear_image(struct pipe_context *ctx) { struct cso_context *cso; struct pipe_resource *cb; - struct pipe_sampler_view *view = NULL; const char *text; cso = cso_create_context(ctx, 0); -- 2.20.0.rc1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Lets talk about autotools
On 27.11.2018 19.05, Matt Turner wrote: > On Tue, Nov 27, 2018 at 1:13 AM Timo Aaltonen wrote: >> >> On 17.11.2018 6.04, Dylan Baker wrote: >>> Quoting Dylan Baker (2018-09-17 09:44:07) I feel like for !windows meson is in good enough shape at this point that we can start having the discussion about deleting the autotools build. So, is there anything left that autotools can do that meson cannot (that we actually want to implement)? And, what is a reasonable time-table to remove the autotools build? I think we could reasonably remove it as soon as 18.3 if others felt confident that it would work for them. Dylan >>> >>> Okay, time for an update on things and a chance to talk about what else we >>> need. >>> >>> Support for llvm-config (and any binary, actually) overriding has landed in >>> meson, and will be present in the 0.49.0 release, which is due out December >>> 9th. >> >> Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and will >> get Mesa backports up until and including 20.0.x, so I wonder how >> complex these required new features in meson are to be backported, or >> perhaps easily worked around? Backporting a whole new version of meson >> might not happen.. > > I understand the LTS concept, but what's the value in never upgrading > something like a build tool like Meson? Yeah, new versions give a > possibility of regressions, but with something evolving as quickly as > Meson the version available in April 2018 becomes less useful for its > intended purpose with each passing month... Fair enough, I'll just package a newer, renamed meson for mesa if necessary. -- t ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 4/5] intel/compiler: Implement unordered comparisons
On 11/22/2018 10:46 AM, Jason Ekstrand wrote: > The vec4 path has only been compile-tested as there's no easy way to > generate a vec4 shader with an unordered equality. > --- > src/intel/compiler/brw_compiler.c | 3 +++ > src/intel/compiler/brw_fs_nir.cpp | 20 +--- > src/intel/compiler/brw_vec4_nir.cpp | 21 + > 3 files changed, 37 insertions(+), 7 deletions(-) > > diff --git a/src/intel/compiler/brw_compiler.c > b/src/intel/compiler/brw_compiler.c > index fe632c5badc..f9e8fa09a34 100644 > --- a/src/intel/compiler/brw_compiler.c > +++ b/src/intel/compiler/brw_compiler.c > @@ -42,6 +42,9 @@ > .lower_fdiv = true, > \ > .lower_flrp64 = true, > \ > .lower_ldexp = true, > \ > + .lower_fltu = true, > \ > + .lower_fgeu = true, > \ Can't we use cmpn.l and cmpn.ge for these? On some platforms it has to be split into two SIMD8 instructions, but that seems better than the lowering... or maybe just use those on BDW+? > + .lower_fne_to_fequ = true, > \ > .lower_cs_local_id_from_index = true, > \ > .lower_device_index_to_zero = true, > \ > .native_integers = true, > \ > diff --git a/src/intel/compiler/brw_fs_nir.cpp > b/src/intel/compiler/brw_fs_nir.cpp > index a62d521bb5d..eba3611e447 100644 > --- a/src/intel/compiler/brw_fs_nir.cpp > +++ b/src/intel/compiler/brw_fs_nir.cpp > @@ -1049,6 +1049,8 @@ fs_visitor::nir_emit_alu(const fs_builder , > nir_alu_instr *instr) > case nir_op_flt: > case nir_op_fge: > case nir_op_feq: > + case nir_op_fne: > + case nir_op_fequ: > case nir_op_fneu: { >fs_reg dest = result; > > @@ -1056,26 +1058,30 @@ fs_visitor::nir_emit_alu(const fs_builder , > nir_alu_instr *instr) >if (bit_size != 32) > dest = bld.vgrf(op[0].type, 1); > > - brw_conditional_mod cond; >switch (instr->op) { >case nir_op_flt: > - cond = BRW_CONDITIONAL_L; > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_L); > break; >case nir_op_fge: > - cond = BRW_CONDITIONAL_GE; > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_GE); > break; >case nir_op_feq: > - cond = BRW_CONDITIONAL_Z; > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_Z); > + break; > + case nir_op_fequ: > + bld.CMP(dest, op[0], op[0], BRW_CONDITIONAL_NZ); > + set_predicate_inv(BRW_PREDICATE_NORMAL, true, /* inverse */ > + bld.CMP(dest, op[1], op[1], BRW_CONDITIONAL_NZ)); > + set_predicate_inv(BRW_PREDICATE_NORMAL, true, /* inverse */ > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_Z)); OpFUnordEqual is (isnan(a) || isnan(b) || a == b), and that's literally what this code does. It seems like this could be implemented (perhaps as a lowering?) as (OpFUnordGreaterThanEqual(a,b) && OpFUnordGreaterThanEqual(b, a)) via cmpn.ge. Using this same technique, that would be 2 instructions (on BDW+) instead of 3. HSW has to do cmpn as two SIMD8 instructions, so this may be better there. Dunno. > break; >case nir_op_fneu: > - cond = BRW_CONDITIONAL_NZ; > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_NZ); > break; >default: > unreachable("bad opcode"); >} > > - bld.CMP(dest, op[0], op[1], cond); > - >if (bit_size > 32) { > bld.MOV(result, subscript(dest, BRW_REGISTER_TYPE_UD, 0)); >} else if(bit_size < 32) { > diff --git a/src/intel/compiler/brw_vec4_nir.cpp > b/src/intel/compiler/brw_vec4_nir.cpp > index f7f46f5034c..32559e1aade 100644 > --- a/src/intel/compiler/brw_vec4_nir.cpp > +++ b/src/intel/compiler/brw_vec4_nir.cpp > @@ -1366,6 +1366,27 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr) >break; > } > > + case nir_op_fequ: { > + dst_reg cmp_res = dst; > + if (nir_src_bit_size(instr->src[0].src) == 64) > + cmp_res = dst_reg(this, glsl_type::dvec4_type); > + > + vec4_instruction *inst; > + inst = emit(CMP(cmp_res, op[0], op[0], BRW_CONDITIONAL_NZ)); > + inst = emit(CMP(cmp_res, op[1], op[1], BRW_CONDITIONAL_NZ)); > + inst->predicate = BRW_PREDICATE_NORMAL; > + inst->predicate_inverse = true; > + inst = emit(CMP(cmp_res, op[0], op[1], BRW_CONDITIONAL_Z)); > + inst->predicate = BRW_PREDICATE_NORMAL; > + inst->predicate_inverse = true; > + > + if (nir_src_bit_size(instr->src[0].src) == 64) { > + dst_reg cmp_res32 =
Re: [Mesa-dev] [RFC 1/5] nir: Rename nir_op_fne to nir_op_fneu
On 11/26/2018 01:05 PM, Eric Anholt wrote: > Jason Ekstrand writes: > >> This way, it's explicit in the opcode name that it's an unordered >> comparison. >> --- >> src/amd/common/ac_nir_to_llvm.c | 2 +- >> src/compiler/glsl/glsl_to_nir.cpp | 4 +- >> src/compiler/nir/nir.h| 2 +- >> src/compiler/nir/nir_builder.h| 2 +- >> src/compiler/nir/nir_loop_analyze.c | 4 +- >> src/compiler/nir/nir_lower_alu_to_scalar.c| 2 +- >> src/compiler/nir/nir_lower_double_ops.c | 6 +-- >> src/compiler/nir/nir_opcodes.py | 2 +- >> src/compiler/nir/nir_opt_algebraic.py | 46 +-- >> src/compiler/spirv/vtn_alu.c | 10 ++-- >> src/compiler/spirv/vtn_glsl450.c | 4 +- >> src/gallium/auxiliary/nir/tgsi_to_nir.c | 4 +- >> .../drivers/freedreno/ir3/ir3_compiler_nir.c | 2 +- >> src/gallium/drivers/vc4/vc4_program.c | 4 +- >> src/intel/compiler/brw_fs_nir.cpp | 4 +- >> src/intel/compiler/brw_vec4_nir.cpp | 4 +- >> 16 files changed, 51 insertions(+), 51 deletions(-) > > Looks like you missed src/broadcom/compiler/ I was going to suggest that this patch should just be a sed job, but... It's a bummer that 'grep -rl nir_op_fne[^g] src/' doesn't produce the full list. It does get src/broadcom/compiler/nir_to_vir.c. > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/compiler: fix register allocation in opt_peephole_sel
Seems obviously correct. Reviewed-by: Ian Romanick On 11/27/2018 01:24 AM, Iago Toral Quiroga wrote: > This wasn't handling 64-bit cases properly. Found by inspection. > --- > src/intel/compiler/brw_fs_sel_peephole.cpp | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/src/intel/compiler/brw_fs_sel_peephole.cpp > b/src/intel/compiler/brw_fs_sel_peephole.cpp > index 4d11d10cc6..98d640a3bf 100644 > --- a/src/intel/compiler/brw_fs_sel_peephole.cpp > +++ b/src/intel/compiler/brw_fs_sel_peephole.cpp > @@ -198,8 +198,7 @@ fs_visitor::opt_peephole_sel() > */ > fs_reg src0(then_mov[i]->src[0]); > if (src0.file == IMM) { > - src0 = vgrf(glsl_type::float_type); > - src0.type = then_mov[i]->src[0].type; > + src0 = ibld.vgrf(then_mov[i]->src[0].type); > ibld.MOV(src0, then_mov[i]->src[0]); > } > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] nir: merge some basic consecutive ifs
On 11/26/2018 09:31 PM, Timothy Arceri wrote: > After trying multiple times to merge if-statements with phis > between them I've come to the conclusion that it cannot be done > without regressions. The problem is for some shaders we end up > with a whole bunch of phis for the merged ifs resulting in > increased register pressure. > > So this patch just merges ifs that have no phis between them. > This seems to be consistent with what LLVM does so for radeonsi > we only see a change (although its a large change) in a single > shader. > > Shader-db results i965 (SKL): > > total instructions in shared programs: 13098176 -> 13098152 (<.01%) > instructions in affected programs: 1326 -> 1302 (-1.81%) > helped: 4 > HURT: 0 > > total cycles in shared programs: 332032989 -> 332037583 (<.01%) > cycles in affected programs: 60665 -> 65259 (7.57%) > helped: 0 > HURT: 4 > > The cycles estimates reported by shader-db for i965 seem inaccurate > as the only difference in the final code is the removal of the > redundent condition evaluations and jumps. The scheduling doesn't even change? Whenever I've encountered things like this, the scheduler has been to blame. It seems surprising that only 4 shaders are affected. Are these all different instances of the same shader? :) > Also the biggest code reduction (~7%) for radeonsi was in a tomb > raider tressfx shader but for some reason this does not get merged > for i965. > > Shader-db results radeonsi (VEGA): > > Totals from affected shaders: > SGPRS: 232 -> 232 (0.00 %) > VGPRS: 164 -> 164 (0.00 %) > Spilled SGPRs: 59 -> 59 (0.00 %) > Spilled VGPRs: 0 -> 0 (0.00 %) > Private memory VGPRs: 0 -> 0 (0.00 %) > Scratch size: 0 -> 0 (0.00 %) dwords per thread > Code Size: 14584 -> 13520 (-7.30 %) bytes > LDS: 0 -> 0 (0.00 %) blocks > Max Waves: 13 -> 13 (0.00 %) > Wait states: 0 -> 0 (0.00 %) > --- > src/compiler/nir/nir_opt_if.c | 93 +++ > 1 file changed, 93 insertions(+) > > diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c > index 62566eb403..dd488b1787 100644 > --- a/src/compiler/nir/nir_opt_if.c > +++ b/src/compiler/nir/nir_opt_if.c > @@ -585,6 +585,98 @@ opt_if_evaluate_condition_use(nir_builder *b, nir_if > *nif) > return progress; > } > > +static void > +simple_merge_if(nir_if *dest_if, nir_if *src_if, bool dest_if_then, > +bool src_if_then) > +{ > + /* Now merge the if branch */ > + nir_block *dest_blk = dest_if_then ? nir_if_last_then_block(dest_if) > + : nir_if_last_else_block(dest_if); > + > + struct exec_list *list = src_if_then ? _if->then_list > +: _if->else_list; > + > + nir_cf_list if_cf_list; > + nir_cf_extract(_cf_list, nir_before_cf_list(list), > + nir_after_cf_list(list)); > + nir_cf_reinsert(_cf_list, nir_after_block(dest_blk)); > +} > + > +static bool > +opt_if_merge(nir_if *nif) > +{ > + bool progress = false; > + > + nir_block *next_blk = nir_cf_node_cf_tree_next(>cf_node); > + if (next_blk && nif->condition.is_ssa) { > + nir_if *next_if = nir_block_get_following_if(next_blk); > + if (next_if && next_if->condition.is_ssa) { > + > + /* Here we merge two consecutive ifs that have the same > + * condition e.g: > + * > + * if ssa_12 { > + * ... > + * } else { > + * ... > + * } > + * if ssa_12 { > + * ... > + * } else { > + * ... > + * } > + * > + * Note: This only merges if-statements when the block between them > + * is empty. The reason we don't try to merge ifs that just have > phis > + * between them is because this can results in increased register > + * pressure. For example when merging if ladders created by indirect > + * indexing. > + */ > + if (nif->condition.ssa == next_if->condition.ssa && > + exec_list_is_empty(_blk->instr_list)) { > + > +simple_merge_if(nif, next_if, true, true); > +simple_merge_if(nif, next_if, false, false); > + > +nir_block *new_then_block = nir_if_last_then_block(nif); > +nir_block *new_else_block = nir_if_last_else_block(nif); > + > +nir_block *old_then_block = nir_if_last_then_block(next_if); > +nir_block *old_else_block = nir_if_last_else_block(next_if); > + > +/* Rewrite the predecessor block for any phis following the > second > + * if-statement. > + */ > +rewrite_phi_predecessor_blocks(next_if, old_then_block, > + old_else_block, > + new_then_block, > + new_else_block); > + > +/* Move phis after merged if to avoid them being
Re: [Mesa-dev] Lets talk about autotools
Hi, On 27.11.2018 19.05, Matt Turner wrote: On Tue, Nov 27, 2018 at 1:13 AM Timo Aaltonen wrote: On 17.11.2018 6.04, Dylan Baker wrote: Quoting Dylan Baker (2018-09-17 09:44:07) I feel like for !windows meson is in good enough shape at this point that we can start having the discussion about deleting the autotools build. So, is there anything left that autotools can do that meson cannot (that we actually want to implement)? And, what is a reasonable time-table to remove the autotools build? I think we could reasonably remove it as soon as 18.3 if others felt confident that it would work for them. Okay, time for an update on things and a chance to talk about what else we need. Support for llvm-config (and any binary, actually) overriding has landed in meson, and will be present in the 0.49.0 release, which is due out December 9th. Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and will get Mesa backports up until and including 20.0.x, so I wonder how complex these required new features in meson are to be backported, or perhaps easily worked around? Backporting a whole new version of meson might not happen.. I understand the LTS concept, but what's the value in never upgrading something like a build tool like Meson? Yeah, new versions give a possibility of regressions, but with something evolving as quickly as Meson the version available in April 2018 becomes less useful for its intended purpose with each passing month... Ubuntu has so called hardware enabling (HWE) packages, which provide newer versions of kernel, mesa and few other components for LTS, and a meta package(s) that pull those in. They're based on package versions tested in development versions of Ubuntu. For example, relevant Ubuntu 18.10 packages would be available as HWE packages for 18.04 somewhere around February, according to preliminary Ubuntu kernel schedule. Of the packages relevant to Mesa, 18.10 has for example: - kernel v4.18 (18.04 has v4.15) - LLVM v7.0 (18.04 has LLVM v6) - Meson v0.47.2 (18.04 has v0.45) I don't know how much that 4 month gap (before new development distro release package versions become available in preceding LTS release as HW packages) fluctuates. Timo? - Eero ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Lets talk about autotools
On Tue, Nov 27, 2018 at 1:13 AM Timo Aaltonen wrote: > > On 17.11.2018 6.04, Dylan Baker wrote: > > Quoting Dylan Baker (2018-09-17 09:44:07) > >> I feel like for !windows meson is in good enough shape at this point that > >> we > >> can start having the discussion about deleting the autotools build. So, is > >> there > >> anything left that autotools can do that meson cannot (that we actually > >> want to > >> implement)? And, what is a reasonable time-table to remove the autotools > >> build? > >> I think we could reasonably remove it as soon as 18.3 if others felt > >> confident > >> that it would work for them. > >> > >> Dylan > > > > Okay, time for an update on things and a chance to talk about what else we > > need. > > > > Support for llvm-config (and any binary, actually) overriding has landed in > > meson, and will be present in the 0.49.0 release, which is due out December > > 9th. > > Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and will > get Mesa backports up until and including 20.0.x, so I wonder how > complex these required new features in meson are to be backported, or > perhaps easily worked around? Backporting a whole new version of meson > might not happen.. I understand the LTS concept, but what's the value in never upgrading something like a build tool like Meson? Yeah, new versions give a possibility of regressions, but with something evolving as quickly as Meson the version available in April 2018 becomes less useful for its intended purpose with each passing month... ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH mesa] wsi/display: fix mem leak when freeing swapchains
Fixes: da997ebec92942193955 "vulkan: Add KHR_display extension using DRM [v10]" Cc: Keith Packard Signed-off-by: Eric Engestrom --- src/vulkan/wsi/wsi_common_display.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/vulkan/wsi/wsi_common_display.c b/src/vulkan/wsi/wsi_common_display.c index cdd84dd720d99d95f291..072f47421db8666cef2f 100644 --- a/src/vulkan/wsi/wsi_common_display.c +++ b/src/vulkan/wsi/wsi_common_display.c @@ -1048,6 +1048,8 @@ wsi_display_swapchain_destroy(struct wsi_swapchain *drv_chain, for (uint32_t i = 0; i < chain->base.image_count; i++) wsi_display_image_finish(drv_chain, >images[i]); + + wsi_swapchain_finish(>base); vk_free(allocator, chain); return VK_SUCCESS; } -- Cheers, Eric ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] egl/wayland: plug memory leak in drm_handle_device()
On Tuesday, 2018-11-27 11:39:42 +, Emil Velikov wrote: > From: Emil Velikov > > As we fail to open the node, we leak the node/device name. > > Cc: mesa-sta...@lists.freedesktop.org > Cc: Eric Engestrom > Signed-off-by: Emil Velikov > --- > src/egl/drivers/dri2/platform_wayland.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/egl/drivers/dri2/platform_wayland.c > b/src/egl/drivers/dri2/platform_wayland.c > index b05f5363163..df25a482908 100644 > --- a/src/egl/drivers/dri2/platform_wayland.c > +++ b/src/egl/drivers/dri2/platform_wayland.c > @@ -1125,6 +1125,8 @@ drm_handle_device(void *data, struct wl_drm *drm, const > char *device) > > dri2_dpy->fd = loader_open_device(dri2_dpy->device_name); > if (dri2_dpy->fd == -1) { > + free(dri2_dpy->device_name); > + dri2_dpy->device_name = NULL: >_eglLog(_EGL_WARNING, "wayland-egl: could not open %s (%s)", >dri2_dpy->device_name, strerror(errno)); You'll want to free *after* printing device_name ;) With that: Reviewed-by: Eric Engestrom >return; > -- > 2.19.1 > > ___ > 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] egl/wayland: plug memory leak in drm_handle_device()
From: Emil Velikov As we fail to open the node, we leak the node/device name. Cc: mesa-sta...@lists.freedesktop.org Cc: Eric Engestrom Signed-off-by: Emil Velikov --- src/egl/drivers/dri2/platform_wayland.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c index b05f5363163..df25a482908 100644 --- a/src/egl/drivers/dri2/platform_wayland.c +++ b/src/egl/drivers/dri2/platform_wayland.c @@ -1125,6 +1125,8 @@ drm_handle_device(void *data, struct wl_drm *drm, const char *device) dri2_dpy->fd = loader_open_device(dri2_dpy->device_name); if (dri2_dpy->fd == -1) { + free(dri2_dpy->device_name); + dri2_dpy->device_name = NULL: _eglLog(_EGL_WARNING, "wayland-egl: could not open %s (%s)", dri2_dpy->device_name, strerror(errno)); return; -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] egl/wayland: bail out when drmGetMagic fails
From: Emil Velikov Currently as the function fails, we pass uninitialized data to the authentication function. Stop doing that and print an warning when the function fails. v2: Plug memory leak in error path (Eric) Cc: mesa-sta...@lists.freedesktop.org Cc: Tapani Pälli (v1) Cc: Eric Engestrom Signed-off-by: Emil Velikov Reviewed-by: Tapani Pälli (v1) Reviewed-by: Eric Engestrom --- Sending out for posterity. --- src/egl/drivers/dri2/platform_wayland.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c index 8122c811288..b05f5363163 100644 --- a/src/egl/drivers/dri2/platform_wayland.c +++ b/src/egl/drivers/dri2/platform_wayland.c @@ -1133,7 +1133,14 @@ drm_handle_device(void *data, struct wl_drm *drm, const char *device) if (drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER) { dri2_dpy->authenticated = true; } else { - drmGetMagic(dri2_dpy->fd, ); + if (drmGetMagic(dri2_dpy->fd, )) { + close(dri2_dpy->fd); + dri2_dpy->fd = -1; + free(dri2_dpy->device_name); + dri2_dpy->device_name = NULL: + _eglLog(_EGL_WARNING, "wayland-egl: drmGetMagic failed"); + return; + } wl_drm_authenticate(dri2_dpy->wl_drm, magic); } } -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Lets talk about autotools
On 27.11.2018 12.20, Erik Faye-Lund wrote: > On Tue, 2018-11-27 at 11:13 +0200, Timo Aaltonen wrote: >> On 17.11.2018 6.04, Dylan Baker wrote: >>> Quoting Dylan Baker (2018-09-17 09:44:07) I feel like for !windows meson is in good enough shape at this point that we can start having the discussion about deleting the autotools build. So, is there anything left that autotools can do that meson cannot (that we actually want to implement)? And, what is a reasonable time-table to remove the autotools build? I think we could reasonably remove it as soon as 18.3 if others felt confident that it would work for them. Dylan >>> >>> Okay, time for an update on things and a chance to talk about what >>> else we need. >>> >>> Support for llvm-config (and any binary, actually) overriding has >>> landed in >>> meson, and will be present in the 0.49.0 release, which is due out >>> December 9th. >> >> Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and >> will >> get Mesa backports up until and including 20.0.x, so I wonder how >> complex these required new features in meson are to be backported, or >> perhaps easily worked around? Backporting a whole new version of >> meson >> might not happen.. >> > > I don't know if this is acceptable or not for packaging, but one could > always install meson from pip, I would assume... Nope, network resources aren't available (nor allowed) when building for the distro. -- t ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium: Fix uninitialized variable warning in compute test.
On Mon, Nov 26, 2018 at 01:13:16PM -0800, Eric Anholt wrote: > The compiler doesn't know that ny != 0, so x might be uninitialized for > the printf at the end. Reviewed-by: Elie Tournier > --- > src/gallium/tests/trivial/compute.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/gallium/tests/trivial/compute.c > b/src/gallium/tests/trivial/compute.c > index afe5d3e9f2be..20e5a4f140c9 100644 > --- a/src/gallium/tests/trivial/compute.c > +++ b/src/gallium/tests/trivial/compute.c > @@ -240,7 +240,7 @@ static void check_tex(struct context *ctx, int slot, >util_format_get_nblocksy(tex->format, tex->height0)); > struct pipe_transfer *xfer; > char *map; > -int x, y, i; > +int x = 0, y, i; > int err = 0; > > if (!check) > -- > 2.20.0.rc1 > > ___ > 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 10/15] anv/android: support creating images from external format
Since we don't know the exact format at creation time, some initialization is done only when bound with memory in vkBindImageMemory. v2: demand dedicated allocation in vkGetImageMemoryRequirements2 if image has external format v3: refactor prepare_ahw_image, support vkBindImageMemory2, calculate stride correctly for rgb(x) surfaces, rename as 'resolve_ahw_image' v4: rebase to b43f955037c changes Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_android.c | 36 ++ src/intel/vulkan/anv_android.h | 8 +++ src/intel/vulkan/anv_android_stubs.c | 10 +++ src/intel/vulkan/anv_device.c| 2 +- src/intel/vulkan/anv_image.c | 103 ++- src/intel/vulkan/anv_private.h | 4 ++ 6 files changed, 160 insertions(+), 3 deletions(-) diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c index bdc720214c4..3bc2b63b1c9 100644 --- a/src/intel/vulkan/anv_android.c +++ b/src/intel/vulkan/anv_android.c @@ -356,6 +356,42 @@ anv_create_ahw_memory(VkDevice device_h, return VK_SUCCESS; } +VkResult +anv_image_from_external( + VkDevice device_h, + const VkImageCreateInfo *base_info, + const struct VkExternalMemoryImageCreateInfo *create_info, + const VkAllocationCallbacks *alloc, + VkImage *out_image_h) +{ + ANV_FROM_HANDLE(anv_device, device, device_h); + + const struct VkExternalFormatANDROID *ext_info = + vk_find_struct_const(base_info->pNext, EXTERNAL_FORMAT_ANDROID); + + if (ext_info && ext_info->externalFormat != 0) { + assert(base_info->format == VK_FORMAT_UNDEFINED); + assert(base_info->imageType == VK_IMAGE_TYPE_2D); + assert(base_info->usage == VK_IMAGE_USAGE_SAMPLED_BIT); + assert(base_info->tiling == VK_IMAGE_TILING_OPTIMAL); + } + + struct anv_image_create_info anv_info = { + .vk_info = base_info, + .isl_extra_usage_flags = ISL_SURF_USAGE_DISABLE_AUX_BIT, + .external_format = true, + }; + + VkImage image_h; + VkResult result = anv_image_create(device_h, _info, alloc, _h); + if (result != VK_SUCCESS) + return result; + + *out_image_h = image_h; + + return VK_SUCCESS; +} + VkResult anv_image_from_gralloc(VkDevice device_h, const VkImageCreateInfo *base_info, diff --git a/src/intel/vulkan/anv_android.h b/src/intel/vulkan/anv_android.h index 01f0e856291..d5c073126e3 100644 --- a/src/intel/vulkan/anv_android.h +++ b/src/intel/vulkan/anv_android.h @@ -29,6 +29,8 @@ #include struct anv_device_memory; +struct anv_device; +struct anv_image; VkResult anv_image_from_gralloc(VkDevice device_h, const VkImageCreateInfo *base_info, @@ -36,6 +38,12 @@ VkResult anv_image_from_gralloc(VkDevice device_h, const VkAllocationCallbacks *alloc, VkImage *pImage); +VkResult anv_image_from_external(VkDevice device_h, + const VkImageCreateInfo *base_info, + const struct VkExternalMemoryImageCreateInfo *create_info, + const VkAllocationCallbacks *alloc, + VkImage *out_image_h); + uint64_t anv_ahw_usage_from_vk_usage(const VkImageCreateFlags vk_create, const VkImageUsageFlags vk_usage); diff --git a/src/intel/vulkan/anv_android_stubs.c b/src/intel/vulkan/anv_android_stubs.c index ccc16500494..a34afc198a1 100644 --- a/src/intel/vulkan/anv_android_stubs.c +++ b/src/intel/vulkan/anv_android_stubs.c @@ -55,3 +55,13 @@ anv_create_ahw_memory(VkDevice device_h, { return VK_ERROR_EXTENSION_NOT_PRESENT; } + +VkResult +anv_image_from_external(VkDevice device_h, +const VkImageCreateInfo *base_info, +const struct VkExternalMemoryImageCreateInfo *create_info, +const VkAllocationCallbacks *alloc, +VkImage *out_image_h) +{ + return VK_ERROR_EXTENSION_NOT_PRESENT; +} diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c index a1ee9315956..5777de96d80 100644 --- a/src/intel/vulkan/anv_device.c +++ b/src/intel/vulkan/anv_device.c @@ -2768,7 +2768,7 @@ void anv_GetImageMemoryRequirements2( switch (ext->sType) { case VK_STRUCTURE_TYPE_MEMORY_DEDICATED_REQUIREMENTS: { VkMemoryDedicatedRequirements *requirements = (void *)ext; - if (image->needs_set_tiling) { + if (image->needs_set_tiling || image->external_format) { /* If we need to set the tiling for external consumers, we need a * dedicated allocation. * diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index 59e9d007831..79777efe456 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -594,6 +594,15 @@ anv_image_create(VkDevice _device, image->drm_format_mod =
[Mesa-dev] [PATCH 07/15] anv: refactor, remove else block in AllocateMemory
This makes it cleaner to introduce more cases where we import memory from different types of external memory buffers. Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_device.c | 64 +++ 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c index 6b5ba25c6bc..0cbbaeca3b3 100644 --- a/src/intel/vulkan/anv_device.c +++ b/src/intel/vulkan/anv_device.c @@ -2340,42 +2340,46 @@ VkResult anv_AllocateMemory( * If the import fails, we leave the file descriptor open. */ close(fd_info->fd); - } else { - const VkExportMemoryAllocateInfoKHR *fd_info = - vk_find_struct_const(pAllocateInfo->pNext, EXPORT_MEMORY_ALLOCATE_INFO_KHR); - if (fd_info && fd_info->handleTypes) - bo_flags |= ANV_BO_EXTERNAL; - - result = anv_bo_cache_alloc(device, >bo_cache, - pAllocateInfo->allocationSize, bo_flags, - >bo); - if (result != VK_SUCCESS) - goto fail; + goto success; + } - const VkMemoryDedicatedAllocateInfoKHR *dedicated_info = - vk_find_struct_const(pAllocateInfo->pNext, MEMORY_DEDICATED_ALLOCATE_INFO_KHR); - if (dedicated_info && dedicated_info->image != VK_NULL_HANDLE) { - ANV_FROM_HANDLE(anv_image, image, dedicated_info->image); + /* Regular allocate (not importing memory). */ - /* Some legacy (non-modifiers) consumers need the tiling to be set on - * the BO. In this case, we have a dedicated allocation. - */ - if (image->needs_set_tiling) { -const uint32_t i915_tiling = - isl_tiling_to_i915_tiling(image->planes[0].surface.isl.tiling); -int ret = anv_gem_set_tiling(device, mem->bo->gem_handle, - image->planes[0].surface.isl.row_pitch_B, - i915_tiling); -if (ret) { - anv_bo_cache_release(device, >bo_cache, mem->bo); - return vk_errorf(device->instance, NULL, -VK_ERROR_OUT_OF_DEVICE_MEMORY, -"failed to set BO tiling: %m"); -} + const VkExportMemoryAllocateInfoKHR *export_info = + vk_find_struct_const(pAllocateInfo->pNext, EXPORT_MEMORY_ALLOCATE_INFO_KHR); + if (export_info && export_info->handleTypes) + bo_flags |= ANV_BO_EXTERNAL; + + result = anv_bo_cache_alloc(device, >bo_cache, + pAllocateInfo->allocationSize, bo_flags, + >bo); + if (result != VK_SUCCESS) + goto fail; + + const VkMemoryDedicatedAllocateInfoKHR *dedicated_info = + vk_find_struct_const(pAllocateInfo->pNext, MEMORY_DEDICATED_ALLOCATE_INFO_KHR); + if (dedicated_info && dedicated_info->image != VK_NULL_HANDLE) { + ANV_FROM_HANDLE(anv_image, image, dedicated_info->image); + + /* Some legacy (non-modifiers) consumers need the tiling to be set on + * the BO. In this case, we have a dedicated allocation. + */ + if (image->needs_set_tiling) { + const uint32_t i915_tiling = +isl_tiling_to_i915_tiling(image->planes[0].surface.isl.tiling); + int ret = anv_gem_set_tiling(device, mem->bo->gem_handle, + image->planes[0].surface.isl.row_pitch_B, + i915_tiling); + if (ret) { +anv_bo_cache_release(device, >bo_cache, mem->bo); +return vk_errorf(device->instance, NULL, + VK_ERROR_OUT_OF_DEVICE_MEMORY, + "failed to set BO tiling: %m"); } } } + success: *pMem = anv_device_memory_to_handle(mem); return VK_SUCCESS; -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/15] anv: support VkExternalFormatANDROID in vkCreateSamplerYcbcrConversion
If external format is used, we store the external format identifier in conversion to be used later when creating VkImageView. v2: rebase to b43f955037c changes Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_formats.c | 13 + 1 file changed, 13 insertions(+) diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c index 972a6f98620..6e7807579df 100644 --- a/src/intel/vulkan/anv_formats.c +++ b/src/intel/vulkan/anv_formats.c @@ -1170,6 +1170,15 @@ VkResult anv_CreateSamplerYcbcrConversion( ANV_FROM_HANDLE(anv_device, device, _device); struct anv_ycbcr_conversion *conversion; + /* Search for VkExternalFormatANDROID and resolve the format. */ + struct anv_format *ext_format = NULL; + const struct VkExternalFormatANDROID *ext_info = + vk_find_struct_const(pCreateInfo->pNext, EXTERNAL_FORMAT_ANDROID); + + uint64_t format = ext_info ? ext_info->externalFormat : 0; + if (format) + ext_format = (struct anv_format *) (uintptr_t) format; + assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_CREATE_INFO); conversion = vk_alloc2(>alloc, pAllocator, sizeof(*conversion), 8, @@ -1190,6 +1199,10 @@ VkResult anv_CreateSamplerYcbcrConversion( conversion->chroma_offsets[1] = pCreateInfo->yChromaOffset; conversion->chroma_filter = pCreateInfo->chromaFilter; + /* Setup external format. */ + if (ext_format) + conversion->format = ext_format; + bool has_chroma_subsampled = false; for (uint32_t p = 0; p < conversion->format->n_planes; p++) { if (conversion->format->planes[p].has_chroma && -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/15] anv: add anv_ahw_usage_from_vk_usage helper function
v2: rebase to b43f955037c changes Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_android.c | 31 src/intel/vulkan/anv_android.h | 2 ++ src/intel/vulkan/anv_android_stubs.c | 7 +++ 3 files changed, 40 insertions(+) diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c index 54b62b9d02f..f2dd16212c1 100644 --- a/src/intel/vulkan/anv_android.c +++ b/src/intel/vulkan/anv_android.c @@ -202,6 +202,37 @@ anv_GetAndroidHardwareBufferPropertiesANDROID( return VK_SUCCESS; } +/* Construct ahw usage mask from image usage bits, see + * 'AHardwareBuffer Usage Equivalence' in Vulkan spec. + */ +uint64_t +anv_ahw_usage_from_vk_usage(const VkImageCreateFlags vk_create, +const VkImageUsageFlags vk_usage) +{ + uint64_t ahw_usage = 0; + + if (vk_usage & VK_IMAGE_USAGE_SAMPLED_BIT) + ahw_usage |= AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE; + + if (vk_usage & VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT) + ahw_usage |= AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE; + + if (vk_usage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) + ahw_usage |= AHARDWAREBUFFER_USAGE_GPU_COLOR_OUTPUT; + + if (vk_create & VK_IMAGE_CREATE_CUBE_COMPATIBLE_BIT) + ahw_usage |= AHARDWAREBUFFER_USAGE_GPU_CUBE_MAP; + + if (vk_create & VK_IMAGE_CREATE_PROTECTED_BIT) + ahw_usage |= AHARDWAREBUFFER_USAGE_PROTECTED_CONTENT; + + /* No usage bits set - set at least one GPU usage. */ + if (ahw_usage == 0) + ahw_usage = AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE; + + return ahw_usage; +} + VkResult anv_image_from_gralloc(VkDevice device_h, const VkImageCreateInfo *base_info, diff --git a/src/intel/vulkan/anv_android.h b/src/intel/vulkan/anv_android.h index a27c364471b..8d748cecc39 100644 --- a/src/intel/vulkan/anv_android.h +++ b/src/intel/vulkan/anv_android.h @@ -34,4 +34,6 @@ VkResult anv_image_from_gralloc(VkDevice device_h, const VkAllocationCallbacks *alloc, VkImage *pImage); +uint64_t anv_ahw_usage_from_vk_usage(const VkImageCreateFlags vk_create, + const VkImageUsageFlags vk_usage); #endif /* ANV_ANDROID_H */ diff --git a/src/intel/vulkan/anv_android_stubs.c b/src/intel/vulkan/anv_android_stubs.c index a6fe5a5e6b3..0671d5635ee 100644 --- a/src/intel/vulkan/anv_android_stubs.c +++ b/src/intel/vulkan/anv_android_stubs.c @@ -32,3 +32,10 @@ anv_image_from_gralloc(VkDevice device_h, { return VK_ERROR_EXTENSION_NOT_PRESENT; } + +uint64_t +anv_ahw_usage_from_vk_usage(const VkImageCreateFlags vk_create, +const VkImageUsageFlags vk_usage) +{ + return 0; +} -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/15] anv/android: add GetAndroidHardwareBufferPropertiesANDROID
Use the anv_format address in formats table as implementation-defined external format identifier for now. When adding YUV format support this might need to change. v2: code cleanup (Jason) v3: set anv_format address as identifier v4: setup suggestedYcbcrModel and suggested[X|Y]ChromaOffset as expected for HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_android.c | 106 + 1 file changed, 106 insertions(+) diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c index 916e76c93ff..54b62b9d02f 100644 --- a/src/intel/vulkan/anv_android.c +++ b/src/intel/vulkan/anv_android.c @@ -29,6 +29,8 @@ #include #include "anv_private.h" +#include "vk_format_info.h" +#include "vk_util.h" static int anv_hal_open(const struct hw_module_t* mod, const char* id, struct hw_device_t** dev); static int anv_hal_close(struct hw_device_t *dev); @@ -96,6 +98,110 @@ anv_hal_close(struct hw_device_t *dev) return -1; } +static VkResult +get_ahw_buffer_format_properties( + VkDevice device_h, + const struct AHardwareBuffer *buffer, + VkAndroidHardwareBufferFormatPropertiesANDROID *pProperties) +{ + ANV_FROM_HANDLE(anv_device, device, device_h); + + /* Get a description of buffer contents . */ + AHardwareBuffer_Desc desc; + AHardwareBuffer_describe(buffer, ); + + /* Verify description. */ + uint64_t gpu_usage = + AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE | + AHARDWAREBUFFER_USAGE_GPU_COLOR_OUTPUT | + AHARDWAREBUFFER_USAGE_GPU_DATA_BUFFER; + + /* "Buffer must be a valid Android hardware buffer object with at least +* one of the AHARDWAREBUFFER_USAGE_GPU_* usage flags." +*/ + if (!(desc.usage & (gpu_usage))) + return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR; + + /* Fill properties fields based on description. */ + VkAndroidHardwareBufferFormatPropertiesANDROID *p = pProperties; + + p->format = vk_format_from_android(desc.format); + + const struct anv_format *anv_format = anv_get_format(p->format); + p->externalFormat = (uint64_t) (uintptr_t) anv_format; + + p->formatFeatures = + anv_get_image_format_features(>info, p->format, +anv_format, VK_IMAGE_TILING_OPTIMAL); + + /* "Images can be created with an external format even if the Android hardware +* buffer has a format which has an equivalent Vulkan format to enable +* consistent handling of images from sources that might use either category +* of format. However, all images created with an external format are subject +* to the valid usage requirements associated with external formats, even if +* the Android hardware buffer’s format has a Vulkan equivalent." +* +* "The formatFeatures member *must* include +* VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT and at least one of +* VK_FORMAT_FEATURE_MIDPOINT_CHROMA_SAMPLES_BIT or +* VK_FORMAT_FEATURE_COSITED_CHROMA_SAMPLES_BIT" +*/ + p->formatFeatures |= + VK_FORMAT_FEATURE_MIDPOINT_CHROMA_SAMPLES_BIT; + + /* "Implementations may not always be able to determine the color model, +* numerical range, or chroma offsets of the image contents, so the values +* in VkAndroidHardwareBufferFormatPropertiesANDROID are only suggestions. +* Applications should treat these values as sensible defaults to use in +* the absence of more reliable information obtained through some other +* means." +*/ + p->samplerYcbcrConversionComponents.r = VK_COMPONENT_SWIZZLE_IDENTITY; + p->samplerYcbcrConversionComponents.g = VK_COMPONENT_SWIZZLE_IDENTITY; + p->samplerYcbcrConversionComponents.b = VK_COMPONENT_SWIZZLE_IDENTITY; + p->samplerYcbcrConversionComponents.a = VK_COMPONENT_SWIZZLE_IDENTITY; + + p->suggestedYcbcrModel = VK_SAMPLER_YCBCR_MODEL_CONVERSION_YCBCR_601; + p->suggestedYcbcrRange = VK_SAMPLER_YCBCR_RANGE_ITU_FULL; + + p->suggestedXChromaOffset = VK_CHROMA_LOCATION_MIDPOINT; + p->suggestedYChromaOffset = VK_CHROMA_LOCATION_MIDPOINT; + + return VK_SUCCESS; +} + +VkResult +anv_GetAndroidHardwareBufferPropertiesANDROID( + VkDevice device_h, + const struct AHardwareBuffer *buffer, + VkAndroidHardwareBufferPropertiesANDROID *pProperties) +{ + ANV_FROM_HANDLE(anv_device, dev, device_h); + struct anv_physical_device *pdevice = >instance->physicalDevice; + + VkAndroidHardwareBufferFormatPropertiesANDROID *format_prop = + vk_find_struct(pProperties->pNext, + ANDROID_HARDWARE_BUFFER_FORMAT_PROPERTIES_ANDROID); + + /* Fill format properties of an Android hardware buffer. */ + if (format_prop) + get_ahw_buffer_format_properties(device_h, buffer, format_prop); + + const native_handle_t *handle = + AHardwareBuffer_getNativeHandle(buffer); + int dma_buf = (handle && handle->numFds) ? handle->data[0] : -1; + if (dma_buf < 0) + return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR; + + /* All memory types. */ +
[Mesa-dev] [PATCH 04/15] anv: add from/to helpers with android and vulkan formats
v2: handle R8G8B8X8 as R8G8B8_UNORM (Jason) v3: add HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL, we make it define for now to avoid direct dependency to minigbm headers Signed-off-by: Tapani Pälli --- src/intel/vulkan/vk_format_info.h | 50 +++ 1 file changed, 50 insertions(+) diff --git a/src/intel/vulkan/vk_format_info.h b/src/intel/vulkan/vk_format_info.h index a1cc6952c8f..555c67704bc 100644 --- a/src/intel/vulkan/vk_format_info.h +++ b/src/intel/vulkan/vk_format_info.h @@ -27,6 +27,56 @@ #include #include +#ifdef ANDROID +#include +/* See i915_private_android_types.h in minigbm. */ +#define HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL 0x100 + +static inline VkFormat +vk_format_from_android(unsigned android_format) +{ + switch (android_format) { + case AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM: + return VK_FORMAT_R8G8B8A8_UNORM; + case AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM: + case AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM: + return VK_FORMAT_R8G8B8_UNORM; + case AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM: + return VK_FORMAT_R5G6B5_UNORM_PACK16; + case AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT: + return VK_FORMAT_R16G16B16A16_SFLOAT; + case AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM: + return VK_FORMAT_A2B10G10R10_UNORM_PACK32; + case HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL: + return VK_FORMAT_G8_B8R8_2PLANE_420_UNORM; + case AHARDWAREBUFFER_FORMAT_BLOB: + default: + return VK_FORMAT_UNDEFINED; + } +} + +static inline unsigned +android_format_from_vk(unsigned vk_format) +{ + switch (vk_format) { + case VK_FORMAT_R8G8B8A8_UNORM: + return AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM; + case VK_FORMAT_R8G8B8_UNORM: + return AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM; + case VK_FORMAT_R5G6B5_UNORM_PACK16: + return AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM; + case VK_FORMAT_R16G16B16A16_SFLOAT: + return AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT; + case VK_FORMAT_A2B10G10R10_UNORM_PACK32: + return AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM; + case VK_FORMAT_G8_B8R8_2PLANE_420_UNORM: + return HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL; + default: + return AHARDWAREBUFFER_FORMAT_BLOB; + } +} +#endif + static inline VkImageAspectFlags vk_format_aspects(VkFormat format) { -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 13/15] anv: support VkSamplerYcbcrConversionInfo in vkCreateImageView
If a conversion struct was passed, then initialize view using format from the conversion structure. v2: use vk_format directly from the anv_format struct Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_image.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index 79777efe456..2ac3eccbbe0 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -1391,6 +1391,16 @@ anv_CreateImageView(VkDevice _device, assert(range->layerCount > 0); assert(range->baseMipLevel < image->levels); + /* Check if a conversion info was passed. */ + const struct anv_format *conv_format = NULL; + const struct VkSamplerYcbcrConversionInfo *conv_info = + vk_find_struct_const(pCreateInfo->pNext, SAMPLER_YCBCR_CONVERSION_INFO); + + if (conv_info) { + ANV_FROM_HANDLE(anv_ycbcr_conversion, conversion, conv_info->conversion); + conv_format = conversion->format; + } + const VkImageViewUsageCreateInfo *usage_info = vk_find_struct_const(pCreateInfo, IMAGE_VIEW_USAGE_CREATE_INFO); VkImageUsageFlags view_usage = usage_info ? usage_info->usage : image->usage; @@ -1435,6 +1445,12 @@ anv_CreateImageView(VkDevice _device, iview->n_planes = anv_image_aspect_get_planes(iview->aspect_mask); iview->vk_format = pCreateInfo->format; + /* Format is undefined, this can happen when using external formats. Set +* view format from the passed conversion info. +*/ + if (iview->vk_format == VK_FORMAT_UNDEFINED && conv_format) + iview->vk_format = conv_format->vk_format; + iview->extent = (VkExtent3D) { .width = anv_minify(image->extent.width , range->baseMipLevel), .height = anv_minify(image->extent.height, range->baseMipLevel), @@ -1451,7 +1467,7 @@ anv_CreateImageView(VkDevice _device, VkImageAspectFlags vplane_aspect = anv_plane_to_aspect(iview->aspect_mask, vplane); struct anv_format_plane format = - anv_get_format_plane(>info, pCreateInfo->format, + anv_get_format_plane(>info, iview->vk_format, vplane_aspect, image->tiling); iview->planes[vplane].image_plane = iplane; -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 12/15] anv: add VkFormat field as part of anv_format
Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_formats.c | 4 src/intel/vulkan/anv_private.h | 1 + 2 files changed, 5 insertions(+) diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c index 6e7807579df..f27456fa41d 100644 --- a/src/intel/vulkan/anv_formats.c +++ b/src/intel/vulkan/anv_formats.c @@ -54,6 +54,7 @@ .aspect = VK_IMAGE_ASPECT_COLOR_BIT, \ }, \ }, \ + .vk_format = __vk_fmt, \ .n_planes = 1, \ } @@ -94,6 +95,7 @@ .aspect = VK_IMAGE_ASPECT_STENCIL_BIT, \ }, \ }, \ + .vk_format = __vk_fmt, \ .n_planes = 2, \ } @@ -102,6 +104,7 @@ .planes = { \ { .isl_format = ISL_FORMAT_UNSUPPORTED, }, \ }, \ + .vk_format = VK_FORMAT_UNDEFINED, \ } #define y_plane(__plane, __hw_fmt, __swizzle, __ycbcr_swizzle, dhs, dvs) \ @@ -127,6 +130,7 @@ .planes = { \ __VA_ARGS__, \ }, \ + .vk_format = __vk_fmt, \ .n_planes = __n_planes, \ .can_ycbcr = true, \ } diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 893c5da7abc..84678de37f8 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -2562,6 +2562,7 @@ struct anv_format_plane { struct anv_format { struct anv_format_plane planes[3]; + VkFormat vk_format; uint8_t n_planes; bool can_ycbcr; }; -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/15] anv: make anv_get_image_format_features public
This will be utilized later by GetAndroidHardwareBufferPropertiesANDROID. Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_formats.c | 22 +++--- src/intel/vulkan/anv_private.h | 5 + 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c index d3a7c2be85c..aa2093f7f15 100644 --- a/src/intel/vulkan/anv_formats.c +++ b/src/intel/vulkan/anv_formats.c @@ -492,11 +492,11 @@ anv_get_format_plane(const struct gen_device_info *devinfo, VkFormat vk_format, // Format capabilities -static VkFormatFeatureFlags -get_image_format_features(const struct gen_device_info *devinfo, - VkFormat vk_format, - const struct anv_format *anv_format, - VkImageTiling vk_tiling) +VkFormatFeatureFlags +anv_get_image_format_features(const struct gen_device_info *devinfo, + VkFormat vk_format, + const struct anv_format *anv_format, + VkImageTiling vk_tiling) { VkFormatFeatureFlags flags = 0; @@ -743,11 +743,11 @@ void anv_GetPhysicalDeviceFormatProperties( *pFormatProperties = (VkFormatProperties) { .linearTilingFeatures = - get_image_format_features(devinfo, vk_format, anv_format, - VK_IMAGE_TILING_LINEAR), + anv_get_image_format_features(devinfo, vk_format, anv_format, + VK_IMAGE_TILING_LINEAR), .optimalTilingFeatures = - get_image_format_features(devinfo, vk_format, anv_format, - VK_IMAGE_TILING_OPTIMAL), + anv_get_image_format_features(devinfo, vk_format, anv_format, + VK_IMAGE_TILING_OPTIMAL), .bufferFeatures = get_buffer_format_features(devinfo, vk_format, anv_format), }; @@ -794,8 +794,8 @@ anv_get_image_format_properties( if (format == NULL) goto unsupported; - format_feature_flags = get_image_format_features(devinfo, info->format, -format, info->tiling); + format_feature_flags = anv_get_image_format_features(devinfo, info->format, +format, info->tiling); switch (info->type) { default: diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index cfe16d2f0d2..c855f9e4b0d 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -3121,6 +3121,11 @@ anv_sanitize_image_offset(const VkImageType imageType, } } +VkFormatFeatureFlags +anv_get_image_format_features(const struct gen_device_info *devinfo, + VkFormat vk_format, + const struct anv_format *anv_format, + VkImageTiling vk_tiling); void anv_fill_buffer_surface_state(struct anv_device *device, struct anv_state state, -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/15] anv/android: support import/export of AHardwareBuffer objects
v2: add support for non-image buffers (AHARDWAREBUFFER_FORMAT_BLOB) v3: properly handle usage bits when creating from image v4: refactor, code cleanup (Jason) v5: rebase to b43f955037c changes, initialize bo flags as ANV_BO_EXTERNAL (Lionel) Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_android.c | 123 +++ src/intel/vulkan/anv_android.h | 10 +++ src/intel/vulkan/anv_android_stubs.c | 16 src/intel/vulkan/anv_device.c| 45 +- src/intel/vulkan/anv_private.h | 5 ++ 5 files changed, 197 insertions(+), 2 deletions(-) diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c index f2dd16212c1..bdc720214c4 100644 --- a/src/intel/vulkan/anv_android.c +++ b/src/intel/vulkan/anv_android.c @@ -233,6 +233,129 @@ anv_ahw_usage_from_vk_usage(const VkImageCreateFlags vk_create, return ahw_usage; } +VkResult +anv_GetMemoryAndroidHardwareBufferANDROID( + VkDevice device_h, + const VkMemoryGetAndroidHardwareBufferInfoANDROID *pInfo, + struct AHardwareBuffer **pBuffer) +{ + ANV_FROM_HANDLE(anv_device_memory, mem, pInfo->memory); + + /* Some quotes from Vulkan spec: +* +* "If the device memory was created by importing an Android hardware +* buffer, vkGetMemoryAndroidHardwareBufferANDROID must return that same +* Android hardware buffer object." +* +* "VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID must +* have been included in VkExportMemoryAllocateInfoKHR::handleTypes when +* memory was created." +*/ + if (mem->ahw) { + *pBuffer = mem->ahw; + /* Increase refcount. */ + AHardwareBuffer_acquire(mem->ahw); + return VK_SUCCESS; + } + + return VK_ERROR_OUT_OF_HOST_MEMORY; +} + +/* + * Called from anv_AllocateMemory when import AHardwareBuffer. + */ +VkResult +anv_import_ahw_memory(VkDevice device_h, + struct anv_device_memory *mem, + const VkImportAndroidHardwareBufferInfoANDROID *info) +{ + ANV_FROM_HANDLE(anv_device, device, device_h); + + /* Import from AHardwareBuffer to anv_device_memory. */ + const native_handle_t *handle = + AHardwareBuffer_getNativeHandle(info->buffer); + + int dma_buf = (handle && handle->numFds) ? handle->data[0] : -1; + if (dma_buf < 0) + return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR; + + uint64_t bo_flags = ANV_BO_EXTERNAL; + if (device->instance->physicalDevice.supports_48bit_addresses) + bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS; + if (device->instance->physicalDevice.use_softpin) + bo_flags |= EXEC_OBJECT_PINNED; + + VkResult result = anv_bo_cache_import(device, >bo_cache, +dma_buf, bo_flags, >bo); + if (result != VK_SUCCESS) + return result; + + /* "If the vkAllocateMemory command succeeds, the implementation must +* acquire a reference to the imported hardware buffer, which it must +* release when the device memory object is freed. If the command fails, +* the implementation must not retain a reference." +*/ + AHardwareBuffer_acquire(info->buffer); + mem->ahw = info->buffer; + + return VK_SUCCESS; +} + +VkResult +anv_create_ahw_memory(VkDevice device_h, + struct anv_device_memory *mem, + const VkMemoryAllocateInfo *pAllocateInfo) +{ + ANV_FROM_HANDLE(anv_device, dev, device_h); + + const VkMemoryDedicatedAllocateInfo *dedicated_info = + vk_find_struct_const(pAllocateInfo->pNext, + MEMORY_DEDICATED_ALLOCATE_INFO); + + uint32_t w = 0; + uint32_t h = 1; + uint32_t layers = 1; + uint32_t format = 0; + uint64_t usage = 0; + + /* If caller passed dedicated information. */ + if (dedicated_info && dedicated_info->image) { + ANV_FROM_HANDLE(anv_image, image, dedicated_info->image); + w = image->extent.width; + h = image->extent.height; + layers = image->array_size; + format = android_format_from_vk(image->vk_format); + usage = anv_ahw_usage_from_vk_usage(image->create_flags, image->usage); + } else if (dedicated_info && dedicated_info->buffer) { + ANV_FROM_HANDLE(anv_buffer, buffer, dedicated_info->buffer); + w = buffer->size; + format = AHARDWAREBUFFER_FORMAT_BLOB; + usage = AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN | + AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN; + } else { + w = pAllocateInfo->allocationSize; + format = AHARDWAREBUFFER_FORMAT_BLOB; + usage = AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN | + AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN; + } + + struct AHardwareBuffer *ahw = NULL; + struct AHardwareBuffer_Desc desc = { + .width = w, + .height = h, + .layers = layers, + .format = format, + .usage = usage, +}; + + if (AHardwareBuffer_allocate(, ) != 0) + return VK_ERROR_OUT_OF_HOST_MEMORY; + + mem->ahw = ahw; + + return VK_SUCCESS; +} +
[Mesa-dev] [PATCH 09/15] anv/android: add ahardwarebuffer external memory properties
v2: have separate memory properties for android, set usage flags for buffers correctly v3: code cleanup (Jason) + limit maxArrayLayers to 1 for AHardwareBuffer based images v4: rebase to b43f955037c changes Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_formats.c | 41 ++ 1 file changed, 41 insertions(+) diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c index aa2093f7f15..972a6f98620 100644 --- a/src/intel/vulkan/anv_formats.c +++ b/src/intel/vulkan/anv_formats.c @@ -973,6 +973,26 @@ static const VkExternalMemoryProperties prime_fd_props = { VK_EXTERNAL_MEMORY_HANDLE_TYPE_DMA_BUF_BIT_EXT, }; +static const VkExternalMemoryProperties android_buffer_props = { + .externalMemoryFeatures = VK_EXTERNAL_MEMORY_FEATURE_EXPORTABLE_BIT | + VK_EXTERNAL_MEMORY_FEATURE_IMPORTABLE_BIT, + .exportFromImportedHandleTypes = + VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID, + .compatibleHandleTypes = + VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID, +}; + + +static const VkExternalMemoryProperties android_image_props = { + .externalMemoryFeatures = VK_EXTERNAL_MEMORY_FEATURE_EXPORTABLE_BIT | + VK_EXTERNAL_MEMORY_FEATURE_IMPORTABLE_BIT | + VK_EXTERNAL_MEMORY_FEATURE_DEDICATED_ONLY_BIT, + .exportFromImportedHandleTypes = + VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID, + .compatibleHandleTypes = + VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID, +}; + VkResult anv_GetPhysicalDeviceImageFormatProperties2( VkPhysicalDevicephysicalDevice, const VkPhysicalDeviceImageFormatInfo2* base_info, @@ -982,6 +1002,7 @@ VkResult anv_GetPhysicalDeviceImageFormatProperties2( const VkPhysicalDeviceExternalImageFormatInfo *external_info = NULL; VkExternalImageFormatPropertiesKHR *external_props = NULL; VkSamplerYcbcrConversionImageFormatProperties *ycbcr_props = NULL; + struct VkAndroidHardwareBufferUsageANDROID *android_usage = NULL; VkResult result; /* Extract input structs */ @@ -1005,6 +1026,9 @@ VkResult anv_GetPhysicalDeviceImageFormatProperties2( case VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_IMAGE_FORMAT_PROPERTIES: ycbcr_props = (void *) s; break; + case VK_STRUCTURE_TYPE_ANDROID_HARDWARE_BUFFER_USAGE_ANDROID: + android_usage = (void *) s; + break; default: anv_debug_ignored_stype(s->sType); break; @@ -1016,6 +1040,15 @@ VkResult anv_GetPhysicalDeviceImageFormatProperties2( if (result != VK_SUCCESS) goto fail; + if (android_usage) { + android_usage->androidHardwareBufferUsage = + anv_ahw_usage_from_vk_usage(base_info->flags, + base_info->usage); + + /* Limit maxArrayLayers to 1 for AHardwareBuffer based images for now. */ + base_props->imageFormatProperties.maxArrayLayers = 1; + } + /* From the Vulkan 1.0.42 spec: * *If handleType is 0, vkGetPhysicalDeviceImageFormatProperties2 will @@ -1029,6 +1062,10 @@ VkResult anv_GetPhysicalDeviceImageFormatProperties2( if (external_props) external_props->externalMemoryProperties = prime_fd_props; break; + case VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID: + if (external_props) +external_props->externalMemoryProperties = android_image_props; + break; default: /* From the Vulkan 1.0.42 spec: * @@ -,6 +1148,10 @@ void anv_GetPhysicalDeviceExternalBufferProperties( case VK_EXTERNAL_MEMORY_HANDLE_TYPE_DMA_BUF_BIT_EXT: pExternalBufferProperties->externalMemoryProperties = prime_fd_props; return; + case VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID: + pExternalBufferProperties->externalMemoryProperties = + android_buffer_props; + return; default: goto unsupported; } -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 15/15] anv/android: turn on VK_ANDROID_external_memory_android_hardware_buffer
Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_extensions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/intel/vulkan/anv_extensions.py b/src/intel/vulkan/anv_extensions.py index 7c81228f705..ed9b2edef32 100644 --- a/src/intel/vulkan/anv_extensions.py +++ b/src/intel/vulkan/anv_extensions.py @@ -69,6 +69,7 @@ MAX_API_VERSION = None # Computed later # the those extension strings, then tests dEQP-VK.api.info.instance.extensions # and dEQP-VK.api.info.device fail due to the duplicated strings. EXTENSIONS = [ +Extension('VK_ANDROID_external_memory_android_hardware_buffer', 3, 'ANDROID'), Extension('VK_ANDROID_native_buffer', 5, 'ANDROID'), Extension('VK_KHR_16bit_storage', 1, 'device->info.gen >= 8'), Extension('VK_KHR_8bit_storage', 1, 'device->info.gen >= 8'), -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 00/15] VK_ANDROID_external_memory_android_hardware_buffer
Hi; Series was rebased and fixes applied from review, also some changes applied to support HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL. With these changes android.graphics.cts.MediaVulkanGpuTest starts to pass, now all tests utilizing AHardwareBuffer pass (CTS + SkQP) \o/ tree: https://cgit.freedesktop.org/~tpalli/mesa/log/?h=ahw android tree used in testing: https://github.com/tpalli/external-mesa/tree/ahw-android CI was happy: https://mesa-ci.01.org/tpalli/builds/642/group/63a9f0ea7bb98050796b649e85481845 Tapani Pälli (15): anv: add create_flags as part of anv_image anv: refactor make_surface to use data from anv_image anv: make anv_get_image_format_features public anv: add from/to helpers with android and vulkan formats anv/android: add GetAndroidHardwareBufferPropertiesANDROID anv: add anv_ahw_usage_from_vk_usage helper function anv: refactor, remove else block in AllocateMemory anv/android: support import/export of AHardwareBuffer objects anv/android: add ahardwarebuffer external memory properties anv/android: support creating images from external format anv: support VkExternalFormatANDROID in vkCreateSamplerYcbcrConversion anv: add VkFormat field as part of anv_format anv: support VkSamplerYcbcrConversionInfo in vkCreateImageView anv: ignore VkSamplerYcbcrConversion on non-yuv formats anv/android: turn on VK_ANDROID_external_memory_android_hardware_buffer src/intel/vulkan/anv_android.c | 296 +++ src/intel/vulkan/anv_android.h | 20 ++ src/intel/vulkan/anv_android_stubs.c | 33 +++ src/intel/vulkan/anv_device.c| 107 +++--- src/intel/vulkan/anv_extensions.py | 1 + src/intel/vulkan/anv_formats.c | 80 +++- src/intel/vulkan/anv_image.c | 200 ++ src/intel/vulkan/anv_private.h | 21 ++ src/intel/vulkan/genX_state.c| 7 +- src/intel/vulkan/vk_format_info.h| 50 + 10 files changed, 731 insertions(+), 84 deletions(-) -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 14/15] anv: ignore VkSamplerYcbcrConversion on non-yuv formats
This fulfills a requirement for clients that want to utilize same code path for images with external formats (VK_FORMAT_UNDEFINED) and "regular" RGBA images where format is known. This is similar to how OES_EGL_image_external works. To support this, we allow color conversion samplers for non-YUV formats but skip setting up conversion when format does not have can_ycbcr flag set. Signed-off-by: Tapani Pälli --- src/intel/vulkan/genX_state.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c index 42800a2581e..9b579d3118c 100644 --- a/src/intel/vulkan/genX_state.c +++ b/src/intel/vulkan/genX_state.c @@ -337,8 +337,11 @@ VkResult genX(CreateSampler)( if (conversion == NULL) break; - sampler->n_planes = conversion->format->n_planes; - sampler->conversion = conversion; + /* Setup conversion only if format is YUV format. */ + if (conversion && conversion->format->can_ycbcr) { +sampler->n_planes = conversion->format->n_planes; +sampler->conversion = conversion; + } break; } #if GEN_GEN >= 9 -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/15] anv: refactor make_surface to use data from anv_image
Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_image.c | 78 ++ src/intel/vulkan/anv_private.h | 5 +++ 2 files changed, 46 insertions(+), 37 deletions(-) diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index 50fa688c052..59e9d007831 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -159,28 +159,26 @@ add_surface(struct anv_image *image, struct anv_surface *surf, uint32_t plane) static bool all_formats_ccs_e_compatible(const struct gen_device_info *devinfo, - const struct VkImageCreateInfo *vk_info) + const VkImageFormatListCreateInfoKHR *fmt_list, + struct anv_image *image) { enum isl_format format = - anv_get_isl_format(devinfo, vk_info->format, - VK_IMAGE_ASPECT_COLOR_BIT, vk_info->tiling); + anv_get_isl_format(devinfo, image->vk_format, + VK_IMAGE_ASPECT_COLOR_BIT, image->tiling); if (!isl_format_supports_ccs_e(devinfo, format)) return false; - if (!(vk_info->flags & VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT)) + if (!(image->create_flags & VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT)) return true; - const VkImageFormatListCreateInfoKHR *fmt_list = - vk_find_struct_const(vk_info->pNext, IMAGE_FORMAT_LIST_CREATE_INFO_KHR); - if (!fmt_list || fmt_list->viewFormatCount == 0) return false; for (uint32_t i = 0; i < fmt_list->viewFormatCount; i++) { enum isl_format view_format = anv_get_isl_format(devinfo, fmt_list->pViewFormats[i], -VK_IMAGE_ASPECT_COLOR_BIT, vk_info->tiling); +VK_IMAGE_ASPECT_COLOR_BIT, image->tiling); if (!isl_formats_are_ccs_e_compatible(devinfo, format, view_format)) return false; @@ -299,11 +297,11 @@ add_aux_state_tracking_buffer(struct anv_image *image, static VkResult make_surface(const struct anv_device *dev, struct anv_image *image, - const struct anv_image_create_info *anv_info, + uint32_t stride, isl_tiling_flags_t tiling_flags, + isl_surf_usage_flags_t isl_extra_usage_flags, VkImageAspectFlagBits aspect) { - const VkImageCreateInfo *vk_info = anv_info->vk_info; bool ok; static const enum isl_surf_dim vk_to_isl_surf_dim[] = { @@ -312,8 +310,7 @@ make_surface(const struct anv_device *dev, [VK_IMAGE_TYPE_3D] = ISL_SURF_DIM_3D, }; - image->extent = anv_sanitize_image_extent(vk_info->imageType, - vk_info->extent); + image->extent = anv_sanitize_image_extent(image->type, image->extent); const unsigned plane = anv_image_aspect_to_plane(image->aspects, aspect); const struct anv_format_plane plane_format = @@ -321,8 +318,8 @@ make_surface(const struct anv_device *dev, struct anv_surface *anv_surf = >planes[plane].surface; const isl_surf_usage_flags_t usage = - choose_isl_surf_usage(vk_info->flags, image->usage, -anv_info->isl_extra_usage_flags, aspect); + choose_isl_surf_usage(image->create_flags, image->usage, +isl_extra_usage_flags, aspect); /* If an image is created as BLOCK_TEXEL_VIEW_COMPATIBLE, then we need to * fall back to linear on Broadwell and earlier because we aren't @@ -332,24 +329,24 @@ make_surface(const struct anv_device *dev, */ bool needs_shadow = false; if (dev->info.gen <= 8 && - (vk_info->flags & VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT) && - vk_info->tiling == VK_IMAGE_TILING_OPTIMAL) { + (image->create_flags & VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT) && + image->tiling == VK_IMAGE_TILING_OPTIMAL) { assert(isl_format_is_compressed(plane_format.isl_format)); tiling_flags = ISL_TILING_LINEAR_BIT; needs_shadow = true; } ok = isl_surf_init(>isl_dev, _surf->isl, - .dim = vk_to_isl_surf_dim[vk_info->imageType], + .dim = vk_to_isl_surf_dim[image->type], .format = plane_format.isl_format, .width = image->extent.width / plane_format.denominator_scales[0], .height = image->extent.height / plane_format.denominator_scales[1], .depth = image->extent.depth, - .levels = vk_info->mipLevels, - .array_len = vk_info->arrayLayers, - .samples = vk_info->samples, + .levels = image->levels, + .array_len = image->array_size, + .samples = image->samples, .min_alignment_B = 0, - .row_pitch_B = anv_info->stride, + .row_pitch_B = stride, .usage = usage, .tiling_flags = tiling_flags); @@ -369,16 +366,16 @@ make_surface(const struct anv_device *dev, assert(tiling_flags == ISL_TILING_LINEAR_BIT); ok = isl_surf_init(>isl_dev, >planes[plane].shadow_surface.isl, - .dim =
[Mesa-dev] [PATCH 01/15] anv: add create_flags as part of anv_image
This will make it possible for next patch to rip anv_image_create_info out from make_surface function. Signed-off-by: Tapani Pälli --- src/intel/vulkan/anv_image.c | 1 + src/intel/vulkan/anv_private.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index 4003ac28444..50fa688c052 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -590,6 +590,7 @@ anv_image_create(VkDevice _device, image->array_size = pCreateInfo->arrayLayers; image->samples = pCreateInfo->samples; image->usage = pCreateInfo->usage; + image->create_flags = pCreateInfo->flags; image->tiling = pCreateInfo->tiling; image->disjoint = pCreateInfo->flags & VK_IMAGE_CREATE_DISJOINT_BIT; image->needs_set_tiling = wsi_info && wsi_info->scanout; diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 62c563294f5..9281479e2e8 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -2670,6 +2670,7 @@ struct anv_image { uint32_t samples; /**< VkImageCreateInfo::samples */ uint32_t n_planes; VkImageUsageFlags usage; /**< Superset of VkImageCreateInfo::usage. */ + VkImageCreateFlags create_flags; /* Flags used when creating image. */ VkImageTiling tiling; /** VkImageCreateInfo::tiling */ /** True if this is needs to be bound to an appropriately tiled BO. -- 2.17.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Lets talk about autotools
On Tue, 2018-11-27 at 11:13 +0200, Timo Aaltonen wrote: > On 17.11.2018 6.04, Dylan Baker wrote: > > Quoting Dylan Baker (2018-09-17 09:44:07) > > > I feel like for !windows meson is in good enough shape at this > > > point that we > > > can start having the discussion about deleting the autotools > > > build. So, is there > > > anything left that autotools can do that meson cannot (that we > > > actually want to > > > implement)? And, what is a reasonable time-table to remove the > > > autotools build? > > > I think we could reasonably remove it as soon as 18.3 if others > > > felt confident > > > that it would work for them. > > > > > > Dylan > > > > Okay, time for an update on things and a chance to talk about what > > else we need. > > > > Support for llvm-config (and any binary, actually) overriding has > > landed in > > meson, and will be present in the 0.49.0 release, which is due out > > December 9th. > > Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and > will > get Mesa backports up until and including 20.0.x, so I wonder how > complex these required new features in meson are to be backported, or > perhaps easily worked around? Backporting a whole new version of > meson > might not happen.. > I don't know if this is acceptable or not for packaging, but one could always install meson from pip, I would assume... ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] winsys/amdgpu: use optimal VM alignment for CPU allocations
Am 27.11.18 um 00:02 schrieb Marek Olšák: From: Marek Olšák --- src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c index 7b239695872..a9170a2bc69 100644 --- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c @@ -1541,22 +1541,24 @@ static struct pb_buffer *amdgpu_bo_from_ptr(struct radeon_winsys *rws, bo = CALLOC_STRUCT(amdgpu_winsys_bo); if (!bo) return NULL; if (amdgpu_create_bo_from_user_mem(ws->dev, pointer, aligned_size, _handle)) goto error; if (amdgpu_va_range_alloc(ws->dev, amdgpu_gpu_va_range_general, - aligned_size, 1 << 12, 0, , _handle, - AMDGPU_VA_RANGE_HIGH)) + aligned_size, + amdgpu_get_optimal_vm_alignment(ws, aligned_size, + ws->info.gart_page_size), + 0, , _handle, AMDGPU_VA_RANGE_HIGH)) For userptrs the VA alignment is most likely irrelevant because they are composed out of 4k pages anyway. On the other hand it shouldn't hurt to handle them the same way. Feel free to add an Acked-by: Christian König to the series. Christian. goto error_va_alloc; if (amdgpu_bo_va_op(buf_handle, 0, aligned_size, va, 0, AMDGPU_VA_OP_MAP)) goto error_va_map; /* Initialize it. */ pipe_reference_init(>base.reference, 1); bo->bo = buf_handle; bo->base.alignment = 0; bo->base.size = size; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Remove unused member variable
Seems already unused at the commit that introduced it .. Reviewed-by: Tapani Pälli On 11/27/18 10:10 AM, Matt Turner wrote: --- src/compiler/glsl/glsl_to_nir.cpp | 4 1 file changed, 4 deletions(-) diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index dc6ffa3599d..61b544fd6d3 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -96,8 +96,6 @@ private: /* most recent deref instruction created */ nir_deref_instr *deref; - nir_variable *var; /* variable created by ir_variable visitor */ - /* whether the IR we're operating on is per-function or global */ bool is_global; @@ -179,7 +177,6 @@ nir_visitor::nir_visitor(nir_shader *shader) _mesa_key_pointer_equal); this->result = NULL; this->impl = NULL; - this->var = NULL; memset(>b, 0, sizeof(this->b)); } @@ -453,7 +450,6 @@ nir_visitor::visit(ir_variable *ir) nir_shader_add_variable(shader, var); _mesa_hash_table_insert(var_table, ir, var); - this->var = var; } ir_visitor_status ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: Call fflush() at the end of nir_print_shader()
Reviewed-by: Tapani Pälli On 11/27/18 10:08 AM, Matt Turner wrote: We normally call with stderr which is unbuffered, so this won't affect that, but it does let me call nir_print_shader(nir, fopen("log", "w+")) from gdb and actually get the whole shader in my file. --- src/compiler/nir/nir_print.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/compiler/nir/nir_print.c b/src/compiler/nir/nir_print.c index 19f26f46405..1dcf043a799 100644 --- a/src/compiler/nir/nir_print.c +++ b/src/compiler/nir/nir_print.c @@ -1311,6 +1311,7 @@ void nir_print_shader(nir_shader *shader, FILE *fp) { nir_print_shader_annotated(shader, fp, NULL); + fflush(fp); } void ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: fix register allocation in opt_peephole_sel
This wasn't handling 64-bit cases properly. Found by inspection. --- src/intel/compiler/brw_fs_sel_peephole.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/intel/compiler/brw_fs_sel_peephole.cpp b/src/intel/compiler/brw_fs_sel_peephole.cpp index 4d11d10cc6..98d640a3bf 100644 --- a/src/intel/compiler/brw_fs_sel_peephole.cpp +++ b/src/intel/compiler/brw_fs_sel_peephole.cpp @@ -198,8 +198,7 @@ fs_visitor::opt_peephole_sel() */ fs_reg src0(then_mov[i]->src[0]); if (src0.file == IMM) { - src0 = vgrf(glsl_type::float_type); - src0.type = then_mov[i]->src[0].type; + src0 = ibld.vgrf(then_mov[i]->src[0].type); ibld.MOV(src0, then_mov[i]->src[0]); } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radv: Clamp gfx9 image view extents to the allocated image extents.
Tested-by: Alex Smith Confirmed it fixes both the testcase and the in-game bug it was causing. Thanks! On Tue, 27 Nov 2018 at 08:34, Samuel Pitoiset wrote: > cc stable? > > Reviewed-by: Samuel Pitoiset > > On 11/24/18 11:31 PM, Bas Nieuwenhuizen wrote: > > Mirrors AMDVLK. Looks like if we go over the alignment of height > > we actually start to change the addressing. Seems like the extra > > miplevels actually work with this. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108245 > > Fixes: f6cc15dccd5 "radv/gfx9: fix block compression texture views. (v2)" > > --- > > src/amd/vulkan/radv_image.c | 6 ++ > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c > > index 7492bf48b51..ba8e28f0e23 100644 > > --- a/src/amd/vulkan/radv_image.c > > +++ b/src/amd/vulkan/radv_image.c > > @@ -1175,8 +1175,6 @@ radv_image_view_init(struct radv_image_view *iview, > >if (device->physical_device->rad_info.chip_class >= GFX9 > && > >vk_format_is_compressed(image->vk_format) && > >!vk_format_is_compressed(iview->vk_format)) { > > - unsigned rounded_img_w = > util_next_power_of_two(iview->extent.width); > > - unsigned rounded_img_h = > util_next_power_of_two(iview->extent.height); > >unsigned lvl_width = > radv_minify(image->info.width , range->baseMipLevel); > >unsigned lvl_height = > radv_minify(image->info.height, range->baseMipLevel); > > > > @@ -1186,8 +1184,8 @@ radv_image_view_init(struct radv_image_view *iview, > >lvl_width <<= range->baseMipLevel; > >lvl_height <<= range->baseMipLevel; > > > > - iview->extent.width = CLAMP(lvl_width, > iview->extent.width, rounded_img_w); > > - iview->extent.height = CLAMP(lvl_height, > iview->extent.height, rounded_img_h); > > + iview->extent.width = CLAMP(lvl_width, > iview->extent.width, iview->image->surface.u.gfx9.surf_pitch); > > + iview->extent.height = CLAMP(lvl_height, > iview->extent.height, iview->image->surface.u.gfx9.surf_height); > >} > > } > > > > > ___ > 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 108245] RADV/Vega: Low mip levels of large BCn textures get corrupted by vkCmdCopyBufferToImage
https://bugs.freedesktop.org/show_bug.cgi?id=108245 Bas Nieuwenhuizen changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 04/11] st/mesa: Use Array._DrawVAO in st_atom_array.c.
On Tue, 2018-11-27 at 07:11 +0100, Mathias Fröhlich wrote: > Hi Erik, > > > > On Monday, 26 November 2018 19:39:50 CET Erik Faye-Lund wrote: > > > > I know this is *very* late notice, but this commit broke Super > > > > Tux > > > > Kart > > > > on VirGL. Both the player-models as as well as the level data > > > > renders > > > > with gibberish vertex positions since this commit. > > > > > > > > The fix that Rob Clark did on top does not fix the problem (and > > > > shouldn't have; VirGL doesn't use NIR). > > > > > > Do you have any idea how I can reproduce that issue with the > > > least > > > effort? > > > > > > > Sadly, no. I run a qemu VM where I run super tux cart. It's a > > rather > > convoluted setup, I'm afraid. If you're interested in that Robert > > Foss > > has written an article about how to set something like this up > > here: > > https://memcpy.io/virtualizing-gpu-access.html > > > > ...But I totally understand if this is asking a bit too much. I can > > help out with any information you need... > > Thanks! > That, just means that looking into has to wait at least until the > weekend. > Probably even later. > > And thanks for looking up the constants. > The effective binding computation depends on these and may change > the set up combined buffer objects. So these are interesting to know. > > I have been putting a lot of internal verification into the code > paths > especially _mesa_update_vao_derived_arrays contains a larger > #ifndef NDEBUG part that may tell us if there is something > unexpected. > > I assume you did run also with asserts enabled in the build? Yes, I ran with asserts on, and none triggered. > I can observe some flicker in supertuxcart on i965. The nvidia blob > seems > not to flicker here. Also when running through valgrind I don't get > that flicker > on i965. Is that flashing - initially looked like a lighting effect > of the game to > me - what you observe too? No, the models are completely garbled. You can find some example screenshots here: https://gitlab.freedesktop.org/virgl/virglrenderer/issues/59 > Also what are the game options? Are shaders enabled in some way? I'm playing with the default settings. I'm not sure what you mean with "are shaders enabled"; VirGL is a gallium-driver, everything uses shaders. > What does change if you change the game settings? > May be that gives us some hints? > I've tried setting both the lowest and highest graphics settings in the game, and I get the same problem. This seems to happen regardless of graphics settings. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Lets talk about autotools
On 17.11.2018 6.04, Dylan Baker wrote: > Quoting Dylan Baker (2018-09-17 09:44:07) >> I feel like for !windows meson is in good enough shape at this point that we >> can start having the discussion about deleting the autotools build. So, is >> there >> anything left that autotools can do that meson cannot (that we actually want >> to >> implement)? And, what is a reasonable time-table to remove the autotools >> build? >> I think we could reasonably remove it as soon as 18.3 if others felt >> confident >> that it would work for them. >> >> Dylan > > Okay, time for an update on things and a chance to talk about what else we > need. > > Support for llvm-config (and any binary, actually) overriding has landed in > meson, and will be present in the 0.49.0 release, which is due out December > 9th. Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and will get Mesa backports up until and including 20.0.x, so I wonder how complex these required new features in meson are to be backported, or perhaps easily worked around? Backporting a whole new version of meson might not happen.. -- t ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 106958] Mass Effect Andromeda renders correctly on RX480 POLARIS but BAD ON RX VEGA 64 on wine 3.10 stagingf with DXVK
https://bugs.freedesktop.org/show_bug.cgi?id=106958 --- Comment #14 from Samuel Pitoiset --- The trace crashes for me as well. Can you record a renderdoc capture instead? I don't have that game, so can't do anything. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radv: Clamp gfx9 image view extents to the allocated image extents.
cc stable? Reviewed-by: Samuel Pitoiset On 11/24/18 11:31 PM, Bas Nieuwenhuizen wrote: Mirrors AMDVLK. Looks like if we go over the alignment of height we actually start to change the addressing. Seems like the extra miplevels actually work with this. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108245 Fixes: f6cc15dccd5 "radv/gfx9: fix block compression texture views. (v2)" --- src/amd/vulkan/radv_image.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c index 7492bf48b51..ba8e28f0e23 100644 --- a/src/amd/vulkan/radv_image.c +++ b/src/amd/vulkan/radv_image.c @@ -1175,8 +1175,6 @@ radv_image_view_init(struct radv_image_view *iview, if (device->physical_device->rad_info.chip_class >= GFX9 && vk_format_is_compressed(image->vk_format) && !vk_format_is_compressed(iview->vk_format)) { -unsigned rounded_img_w = util_next_power_of_two(iview->extent.width); -unsigned rounded_img_h = util_next_power_of_two(iview->extent.height); unsigned lvl_width = radv_minify(image->info.width , range->baseMipLevel); unsigned lvl_height = radv_minify(image->info.height, range->baseMipLevel); @@ -1186,8 +1184,8 @@ radv_image_view_init(struct radv_image_view *iview, lvl_width <<= range->baseMipLevel; lvl_height <<= range->baseMipLevel; - iview->extent.width = CLAMP(lvl_width, iview->extent.width, rounded_img_w); -iview->extent.height = CLAMP(lvl_height, iview->extent.height, rounded_img_h); +iview->extent.width = CLAMP(lvl_width, iview->extent.width, iview->image->surface.u.gfx9.surf_pitch); +iview->extent.height = CLAMP(lvl_height, iview->extent.height, iview->image->surface.u.gfx9.surf_height); } } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Please bring back __GL_FSAA_MODE
Hello, I realise this was removed for a reason ( https://lists.freedesktop.org/archives/mesa-dev/2014-September/067864.html ) but there are cases where it is useful. In older games that do not support native FSAA being able to force it in the driver is the only way to enable it. A quick google search for AMD linux force msaa shows that I'm not the only one who would like to see this feature return: https://www.phoronix.com/forums/forum/linux-graphics-x-org-drivers/open-source-amd-linux/1024166-radeon-eqaa-anti-aliasing-support-merged-to-mesa-18-2?p=1024185#post1024185 https://bbs.archlinux.org/viewtopic.php?id=225425 https://www.reddit.com/r/linux_gaming/comments/671yzm/forcing_antialiasing_with_mesa_radeon_driver/ https://github.com/dscharrer/void/blob/master/hacks/forcemsaa.c I understand it does cause issues in some cases but there are times when it is useful. Could it be reintroduced with a more relevant name that implies it shouldn't be used? E.g. GALLIUM_LEGACY_MSAA or GALLIUM_FORCE_MSAA_EXPERIMENTAL. That way it would lower users expectations while still making the option available to try. Kind Regards, Tom Butler ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: Remove unused member variable
--- src/compiler/glsl/glsl_to_nir.cpp | 4 1 file changed, 4 deletions(-) diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index dc6ffa3599d..61b544fd6d3 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -96,8 +96,6 @@ private: /* most recent deref instruction created */ nir_deref_instr *deref; - nir_variable *var; /* variable created by ir_variable visitor */ - /* whether the IR we're operating on is per-function or global */ bool is_global; @@ -179,7 +177,6 @@ nir_visitor::nir_visitor(nir_shader *shader) _mesa_key_pointer_equal); this->result = NULL; this->impl = NULL; - this->var = NULL; memset(>b, 0, sizeof(this->b)); } @@ -453,7 +450,6 @@ nir_visitor::visit(ir_variable *ir) nir_shader_add_variable(shader, var); _mesa_hash_table_insert(var_table, ir, var); - this->var = var; } ir_visitor_status -- 2.18.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/fs: Handle V/UV immediates in dump_instructions()
--- src/intel/compiler/brw_fs.cpp | 5 + 1 file changed, 5 insertions(+) diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp index e030f7215ce..c3a09d31938 100644 --- a/src/intel/compiler/brw_fs.cpp +++ b/src/intel/compiler/brw_fs.cpp @@ -6036,6 +6036,11 @@ fs_visitor::dump_instruction(backend_instruction *be_inst, FILE *file) brw_vf_to_float((inst->src[i].ud >> 16) & 0xff), brw_vf_to_float((inst->src[i].ud >> 24) & 0xff)); break; + case BRW_REGISTER_TYPE_V: + case BRW_REGISTER_TYPE_UV: +fprintf(file, "%08x%s", inst->src[i].ud, +inst->src[i].type == BRW_REGISTER_TYPE_V ? "V" : "UV"); +break; default: fprintf(file, "???"); break; -- 2.18.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nir: Call fflush() at the end of nir_print_shader()
We normally call with stderr which is unbuffered, so this won't affect that, but it does let me call nir_print_shader(nir, fopen("log", "w+")) from gdb and actually get the whole shader in my file. --- src/compiler/nir/nir_print.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/compiler/nir/nir_print.c b/src/compiler/nir/nir_print.c index 19f26f46405..1dcf043a799 100644 --- a/src/compiler/nir/nir_print.c +++ b/src/compiler/nir/nir_print.c @@ -1311,6 +1311,7 @@ void nir_print_shader(nir_shader *shader, FILE *fp) { nir_print_shader_annotated(shader, fp, NULL); + fflush(fp); } void -- 2.18.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev