Re: [Mesa-dev] [PATCH] i965: Don't check for draw-time errors that cannot occur in core profile
Ilia Mirkin writes: > On Tue, Sep 1, 2015 at 1:48 AM, Eirik Byrkjeflot Anonsen > wrote: >> Ian Romanick writes: >> >>> ping. :) >>> >>> On 08/10/2015 11:48 AM, Matt Turner wrote: On Mon, Aug 10, 2015 at 10:12 AM, Ian Romanick wrote: > From: Ian Romanick > > On many CPU-limited applications, this is *the* hot path. The idea is > to generate per-API versions of brw_draw_prims that elide some checks. > This patch removes render-mode and "is everything in VBOs" checks from > core-profile contexts. > > On my IVB laptop (which may have experienced thermal throttling): > > Gl32Batch7: 3.70955% +/- 1.11344% I'm getting 3.18414% +/- 0.587956% (n=113) on my IVB, , which probably matches your numbers depending on your value of n. > OglBatch7: 1.04398% +/- 0.772788% I'm getting 1.15377% +/- 1.05898% (n=34) on my IVB, which probably matches your numbers depending on your value of n. >>> >>> This is another thing that make me feel a little uncomfortable with the >>> way we've done performance measurements in the past. If I run my test >>> before and after this patch for 121 iterations, which I have done, I can >>> cut the data at any point and oscillate between "no difference" or X% >>> +/- some-large-fraction-of-X%. Since the before and after code for the >>> compatibility profile path should be identical, "no difference" is the >>> only believable result. >> >> That's pretty much expected, I believe. In essence, you are running 121 >> tests, each with a 95% confidence interval and so should expect >> somewhere around 5 "significant difference" results. That's not entirely >> true of course, since these are not 121 *independent* tests, but the >> basic problem remains. > > (more stats rants follow) :) > While my job title has never been 'statistician', I've been around a > bunch of them. Just want to correct this... let's forget about these > tests, but instead think about coin flips (of a potentially unfair > coin). What you're doing is flipping the coin 100 times, and then > looking at the number of times it came up heads and tails. From that > you're inferring the mean of the distribution. [...] (I have a background in mathematics with a small amount of both probability theory and statistics, but I haven't really worked with it, so your background may make you more of a "statistician" than me :) ) I think what Ian was saying was that he was flipping the coin 100 times and then after every flip checking whether the result so far suggested a 50/50 result (fair coin) or not. And he found that sometimes during the run he would get a "fair coin" result, which he thought was in conflict with the final "loaded coin" result. Thus he was questioning whether the final "loaded coin" result was correct. I was simplifying heavily to point out the problem with this particular way of looking at the result. Now, if I understood Ian's comment correctly, his main reason for doubting the "loaded coin" result was that he thought there was no code differences to explain the result. Which is a very good reason to suspect a problem somewhere. I'm just saying that looking at partial results of a full test run doesn't invalidate the result of the full test run. More importantly, looking at partial results of a test run does not provide any more information on the "truth" than the result of the full test run (again a simplification, but only if you really know what you're doing). eirik ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC] Fix for clang compiler issue as reported in Bug 91826
Clang tryed to declare the non type member of struct module (enum type type)
(in clover/core/module.hpp) instead of a variable of type enum (enum type).
Signed-off-by: Albert Freeman
---
src/gallium/state_trackers/clover/llvm/invocation.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
b/src/gallium/state_trackers/clover/llvm/invocation.cpp
index 7c23a27..d74b50d 100644
--- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
+++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
@@ -465,7 +465,7 @@ namespace {
const bool is_write_only = access_qual == "write_only";
const bool is_read_only = access_qual == "read_only";
-typename module::argument::type marg_type;
+enum module::argument::type marg_type;
if (is_image2d && is_read_only) {
marg_type = module::argument::image2d_rd;
} else if (is_image2d && is_write_only) {
--
2.5.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Revert "i965: Stop aux data compare preventing program binary re-use"
On Fri, Aug 28, 2015 at 09:34:12AM -0700, Ben Widawsky wrote:
> On Fri, Aug 28, 2015 at 10:15:30AM +0300, Pohjolainen, Topi wrote:
> > On Fri, Aug 28, 2015 at 09:56:43AM +0300, Pohjolainen, Topi wrote:
> > > On Thu, Aug 27, 2015 at 10:05:14AM -0700, Ben Widawsky wrote:
> > > > On Thu, Aug 27, 2015 at 10:51:59AM +0300, Pohjolainen, Topi wrote:
> > > > > On Wed, Aug 26, 2015 at 03:46:05PM -0700, Ben Widawsky wrote:
> > > > > > This reverts commit 1bba29ed403e735ba0bf04ed8aa2e571884fcaaf
> > > > > > Author: Topi Pohjolainen
> > > > > > Date: Thu Jun 25 14:00:41 2015 +0300
> > > > > >
> > > > > > i965: Stop aux data compare preventing program binary re-use
> > > > > >
> > > > > > This fixes an intermittent failure in
> > > > > > piglit.spec.arb_pixel_buffer_object.texsubimage pbo.sklm64 (maybe
> > > > > > other
> > > > > > platforms as well, but it is harder to reproduce). I can usually
> > > > > > hit the failure
> > > > > > within 10 runs of the test. This is a very hairy commit to debug.
> > > > > > I'll let Topi
> > > > > > handle it, or else we should go with the revert. I am open to
> > > > > > either. I got
> > > > > > lucky that Jenkins caught this on a run.
> > > > > >
> > > > > > Here was the script I used for bisect:
> > > > > >
> > > > > > i=0
> > > > > > while [ $i -lt 40 ] ; do
> > > > > >./bin/texsubimage pbo -auto -fbo > /dev/null 2>&1
> > > > > >[[ $? != 0 ]] && echo fail && exit 1
> > > > > >((i++))
> > > > > > done
> > > > > >
> > > > > > exit 0
> > > > >
> > > > > Should I use different piglit version than the current master? I'm
> > > > > asking
> > > > > because I get this with both Mesa master and my patch reverted.
> > > >
> > > > My piglit was pretty old. I just updated that, but mesa was master as of
> > > > yesterday (output is below)
> > > >
> > > > The one bit of advice I can add is that you make sure your system is
> > > > updated to
> > > > the very latest.
> > > >
> > > > >
> > > > > testrunner@skl-y:~/topi/piglit$ ./bin/texsubimage pbo -auto -fbo
> > > > > Using test set: Core formats
> > > > > texsubimage failed
> > > > > target: GL_TEXTURE_2D
> > > > > internal format: GL_COMPRESSED_RGB_S3TC_DXT1_EXT
> > > > > region: 68, 12 32 x 48
> > > > > texsubimage failed
> > > > > target: GL_TEXTURE_2D
> > > > > internal format: GL_COMPRESSED_RGBA_S3TC_DXT1_EXT
> > > > > region: 0, 28 116 x 20
> > > > > texsubimage failed
> > > > > target: GL_TEXTURE_2D
> > > > > internal format: GL_COMPRESSED_RGBA_S3TC_DXT3_EXT
> > > > > region: 16, 4 60 x 36
> > > > > texsubimage failed
> > > > > target: GL_TEXTURE_2D
> > > > > internal format: GL_COMPRESSED_RGBA_S3TC_DXT5_EXT
> > > > > region: 8, 0 104 x 60
> > > > > Mesa: User error: GL_INVALID_OPERATION in glTexSubImage2D(out of
> > > > > bounds PBO access)
> > > > > PIGLIT: {"result": "fail" }
> > > >
> > > > Interesting. It so happens I have a patch that purports to fix some of
> > > > these
> > > > things. I did not try this patch myself for this issue.
> > > > http://patchwork.freedesktop.org/patch/54025/
> > > >
> > > > (I've been hanging on to this since I needed to do a bit of research to
> > > > address
> > > > Matt's feedback, specifically regarding render compression).
> > > >
> > > > >
> > > > >
> > > > > Could you include the console output of the failure you get?
> > > > >
> > > >
> > > > Here are two sample failures (occurred in 7 runs)
> > > >
> > > > Using test set: Core formats
> > > > texsubimage failed
> > > > target: GL_TEXTURE_2D
> > > > internal format: GL_INTENSITY
> > > > region: 27, 1 13 x 61
> > > > Mesa: User error: GL_INVALID_OPERATION in glTexSubImage3D(out of bounds
> > > > PBO access)
> > > >
> > > > Using test set: Core formats
> > > > texsubimage failed
> > > > target: GL_TEXTURE_2D
> > > > internal format: GL_INTENSITY
> > > > region: 80, 38 24 x 25
> > > > texsubimage failed
> > > > target: GL_TEXTURE_2D
> > > > internal format: GL_LUMINANCE8
> > > > region: 50, 5 26 x 58
> > > > Mesa: User error: GL_INVALID_OPERATION in glTexSubImage2D(out of bounds
> > > > PBO access)
> > > > PIGLIT: {"result": "fail" }
> > > >
> > > >
> > > > > >
> > > > > > Cc:
> > > > > > Cc: Kenneth Graunke
> > > > > > Cc: Topi Pohjolainen
> > > > > > Reported-by: Mark Janes (jenkins)
> > > > > > Signed-off-by: Ben Widawsky
> > > > > > ---
> > > > > > src/mesa/drivers/dri/i965/brw_state_cache.c | 52
> > > > > > ++---
> > > > > > 1 file changed, 32 insertions(+), 20 deletions(-)
> > > > > >
> > > > > > diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c
> > > > > > b/src/mesa/drivers/dri/i965/brw_state_cache.c
> > > > > > index fbc0419..e50d6a0 100644
> > > > > > --- a/src/mesa/drivers/dri/i965/brw_state_cache.c
> > > > > > +++ b/src/mesa/drivers/dri/i965/brw_state_cache.c
> > > > > > @@ -200,23 +200,36 @@ brw_cache_new_bo(struct brw_cache *cache,
> > > > > > uint32_t new_size)
> > > > > > }
> > > > >
Re: [Mesa-dev] [RFC] Fix for clang compiler issue as reported in Bug 91826
On Tuesday 01 September 2015 17:10:33 Albert Freeman wrote:
> Clang tryed to declare the non type member of struct module (enum type type)
> (in clover/core/module.hpp) instead of a variable of type enum (enum type).
>
> Signed-off-by: Albert Freeman
Reviewed by Serge Martin
Can it be pushed to master, and if so, can it also be pushed to 11.0 branch?
Serge
> ---
> src/gallium/state_trackers/clover/llvm/invocation.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp index
> 7c23a27..d74b50d 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -465,7 +465,7 @@ namespace {
> const bool is_write_only = access_qual == "write_only";
> const bool is_read_only = access_qual == "read_only";
>
> -typename module::argument::type marg_type;
> +enum module::argument::type marg_type;
> if (is_image2d && is_read_only) {
> marg_type = module::argument::image2d_rd;
> } else if (is_image2d && is_write_only) {
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91130] Mesa's cl.h defines CL_VERSION_1_2 even though it is missing some OpenCL 1.2 functions
https://bugs.freedesktop.org/show_bug.cgi?id=91130 Serge Martin changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #5 from Serge Martin --- This should by fix by a97f1b697b01dca9f72d8559f8269188d76dccc9 : clover: Stub missing CL 1.2 functions. -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] meta: Compute correct buffer size with SkipRows/SkipPixels
On Mon, Aug 31, 2015 at 10:29:51PM -0700, Jason Ekstrand wrote: >On Aug 31, 2015 6:48 AM, "Chris Wilson" <[1]ch...@chris-wilson.co.uk> >wrote: >> >> From: Jason Ekstrand <[2]jason.ekstr...@intel.com> >> >> If the user is specifying a subregion of a buffer using SKIP_ROWS and >> SKIP_PIXELS, we must compute the buffer size carefully as the end of the >> last row may be much shorter than stride*image_height*depth. The current >> code tries to memcpy from beyond the end of the user data, for example >> causing: >> Fixes: 7f396189f073d626c5f7a2c232dac92b65f5a23f > >How can this fix a SHA? What repo is this SHA in? I suppose it is an actual regression from commit 7f396189f073d626c5f7a2c232dac92b65f5a23f Author: Jason Ekstrand Date: Mon Jan 5 18:17:04 2015 -0800 meta: Add a BlitFramebuffers-based implementation of TexSubImage >> if (is_pixel_pack) >> - _mesa_BufferData(pbo_target, row_stride * height, NULL, >> + _mesa_BufferData(pbo_target, >> + last_pixel - first_pixel, >> + NULL, > >I'm trying to decide if the null here is correct... It's certainly the >same as what we had before but I have a nagging feeling that this should >be (void *)first_pixel. It should be NULL, but the code doesn't work for the GetTexSubImage case as it does a GPU copy into the buffer object that it creates but then never reads it back from the buffer object into the client memory. Fortunately the non-pbo path is not used for GetTexImage. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] r600g: Simplify out a couple of unnecessary branches
From: Edward O'Callaghan Signed-off-by: Edward O'Callaghan --- src/gallium/drivers/r600/r600_shader.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c index b7d7828..1ab389c 100644 --- a/src/gallium/drivers/r600/r600_shader.c +++ b/src/gallium/drivers/r600/r600_shader.c @@ -1966,13 +1966,9 @@ static int r600_shader_from_tgsi(struct r600_context *rctx, ctx.nliterals = 0; ctx.literals = NULL; - shader->fs_write_all = FALSE; - if (ctx.info.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS]) - shader->fs_write_all = TRUE; - shader->vs_position_window_space = FALSE; - if (ctx.info.properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION]) - shader->vs_position_window_space = TRUE; + shader->fs_write_all = ctx.info.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS]; + shader->vs_position_window_space = ctx.info.properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION]; if (shader->vs_as_gs_a) vs_add_primid_output(&ctx, key.vs.prim_id_out); -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/7] NIR dead control flow, take two
On Tuesday, July 21, 2015 09:53:25 PM Connor Abbott wrote: > This is another spin of my dead control flow elimination series, this > time based on the control flow modification series I sent out earlier. > It's much shorter, since it now uses the much-more-reusable modification > API, rather than a few ad-hoc functions to do what it needs to do. The > changes themselves were relatively minor, however. > > Like before, the last patch is for testing purposes only; it lobotomizes > the GLSL IR pass that does roughly the equivalent thing, so that the NIR > version gets more exposure. It causes one extra piglit failure, but > that's expected and not due to the NIR pass. The constant folding test > expects the compiler to optimize away a function call before link time, > but after disabling the optimization, that no longer happens. Jenkins > reports that piglit.spec.arb_shader_atomic_counters.semantics also > fails, but only on one platform and I couldn't reproduce it so it seems > like a spurious failure. Other than that, there are no piglit > regressions. > > Connor Abbott (7): > nir: add an optimization for removing dead control flow > nir/dead_cf: delete code that's unreachable due to jumps > nir: add nir_block_get_following_loop() helper > nir: add a helper for iterating over blocks in a cf node > nir/dead_cf: add support for removing useless loops > i965/fs/nir: enable the dead control flow optimization > XXX disable opt_if_simplification > > src/glsl/Makefile.sources | 1 + > src/glsl/glsl_parser_extras.cpp | 2 +- > src/glsl/nir/nir.c | 23 +++ > src/glsl/nir/nir.h | 6 + > src/glsl/nir/nir_opt_dead_cf.c | 359 > > src/mesa/drivers/dri/i965/brw_nir.c | 2 + > 6 files changed, 392 insertions(+), 1 deletion(-) > create mode 100644 src/glsl/nir/nir_opt_dead_cf.c > > Series (other than the XXX patch) is: Reviewed-by: Kenneth Graunke I've also gone ahead and pushed it to master. My only concern was that I think we could do this more cleanly...but I haven't come up with anything better. It appears correct, and does really useful things. So, landed. Thanks for writing this! signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/8] nouveau: add support for vaapi
Hi, Thx for the comments. I forgot to mention that there is nothing really new in these patches. All the low level code relative to nouveau was already there. The patches are almost "just" moving code in order to call nouveau_vp3_bsp and nvc0_decoder_bsp multiple times between each begin/end frame. But any refactoring is a source of regressions so the "just" may be not that trivial :) I made sure that vdpau was still working. Actually it was more than that, without vdpau I think I would not have been able to do this refactoring. I also compared that for one begin/end frame the nouveau_bo buffer contains the same data for vdpau and vaapi. I dumped them for the same stream and several seconds and there were all the same. Would it be ok if I squash the patches like this (4 patches instead of 8): *A: Common to nvc0 and nv98:* nouveau: extract memcpy loop from nouveau_vp3_bsp nouveau: remove nouveau_vp3_bsp to use begin/next/end *B: Only for nvc0 but same split can be applied to nv98:* +nvc0(-nouveau): split nvc0_decoder_bsp in begin/next/end +nvc0(-nouveau): remove nvc0_decoder_bsp and use begin/next/end instead +nvc0(-nouveau): preserve content buffer when calling nvc0_decoder_bsp_next nvc0: implement pipe_video_codec::begin_frame/end_frame *C: Common to nvc0 and nv98:* nouveau: fix chunk decoding by updating number of slices *D: Common to nvc0 and nv98:* build: enable st/va with nouveau driver I can still squash A and B, no pb. Let me know. Also should I use "--in-reply-to" or should I submit a new patch set since some will be squashed ? Cheers Julien On 27 August 2015 at 22:36, Emil Velikov wrote: > On 27 August 2015 at 18:35, Ilia Mirkin wrote: > > On Thu, Aug 27, 2015 at 1:11 PM, Emil Velikov > wrote: > >> HI Julien, > >> > >> On 27 August 2015 at 15:15, Julien Isorce wrote: > >>> Currently nouveau does not support chunk decoding > >>> which is required to support st/va > >>> > >>> The following patches refactor nouveau_vp3_bsp > >>> and nvc0_decoder_bsp in order to implement > >>> pipe_video_codec::begin_frame/decode_bitstream/end_frame. > >>> So that decode_bitstream can be call multiple times > >>> between each begin/end. > >>> > >>> TODO: apply same logic for nv98 but I do not have the > >>> material to test it. > >>> > >>> https://bugs.freedesktop.org/show_bug.cgi?id=89969 > >>> > >>> Julien Isorce (8): > >>> nouveau: extract memcpy loop from nouveau_vp3_bsp > >>> nouveau: remove nouveau_vp3_bsp to use begin/next/end > >>> nouveau: split nvc0_decoder_bsp in begin/next/end > >>> nouveau: preserve content buffer when calling nvc0_decoder_bsp_next > >>> nouveau: remove nvc0_decoder_bsp and use begin/next/end instead > >>> nvc0: implement pipe_video_codec::begin_frame/end_frame > >>> nouveau: fix chunk decoding by updating number of slices > >>> build: enable st/va with nouveau driver > >>> > >> Glad to hear that you've got vaapi working. Did you check that vdpau > >> did not regress ? > >> > >> I'm wondering if it's going to be better if you gradually re-factor > >> the existing code. The current "1. add 'new' implementation (incl. > >> variables we don't want/need until halfway through), 2. remove 'old' > >> implementation" approach does not seem too appealing imho. > >> > >> Others might disagree with my suggestion, though :) > > > > I wholeheartedly agree... the current patch series makes it very > > difficult to verify that you're doing stuff "right" since I have to > > look at the "add" and "remove" patches at once. Also your "preserve > > content buffer" thing should probably be folded into the previous > > patch, since that's integral to what those functions do. I realize > > that in the end it'll end up creating just one or two mega-patches, > > but I'd rather have that than the functionality scattered all over. > > > Actually it seems that one can get away without big patches. > Two begin/next/end combos = 6 patches, a cleanup/fix or two plus the > enable patch. > > > You don't have to worry about nv98_video if you don't want to, I can > > take care of fixing that up afterwards, but do try to leave as much > > functionality in nouveau_vp3 as possible. (Perhaps you've done that, I > > haven't looked in great detail yet.) > > > From a quick look all the nouveau_vp3 code is still there. > > -Emil > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91826] Mesa 11.0.0-rc2 - state_trackers/clover does not build on FreeBSD
https://bugs.freedesktop.org/show_bug.cgi?id=91826 --- Comment #5 from Koop Mast --- The patch from #3 fixes the build with clang, thanks. Report what to clang upstream, that they error on this issue versus gcc which only gives a warning? -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nir: UBO loads no longer use const_index[1]
Commit 2126c68e5cba killed the array elements parameter on load/store intrinsics that was stored in const_index[1]. It looks like that patch missed to remove this assignment in the UBO path. --- src/glsl/nir/glsl_to_nir.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp index 5fb4ee2..0712908 100644 --- a/src/glsl/nir/glsl_to_nir.cpp +++ b/src/glsl/nir/glsl_to_nir.cpp @@ -1001,7 +1001,6 @@ nir_visitor::visit(ir_expression *ir) nir_intrinsic_instr *load = nir_intrinsic_instr_create(this->shader, op); load->num_components = ir->type->vector_elements; load->const_index[0] = const_index ? const_index->value.u[0] : 0; /* base offset */ - load->const_index[1] = 1; /* number of vec4's */ load->src[0] = evaluate_rvalue(ir->operands[0]); if (!const_index) load->src[1] = evaluate_rvalue(ir->operands[1]); -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91826] Mesa 11.0.0-rc2 - state_trackers/clover does not build on FreeBSD
https://bugs.freedesktop.org/show_bug.cgi?id=91826 --- Comment #6 from Albert Freeman --- Yeah, I don't know too much about C++11 but it MIGHT be a bug in clang. On 1 September 2015 at 09:30, wrote: > Comment # 5 on bug 91826 from Koop Mast > > The patch from #3 fixes the build with clang, thanks. > > Report what to clang upstream, that they error on this issue versus gcc > which > only gives a warning? > > > 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 > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91826] Mesa 11.0.0-rc2 - state_trackers/clover does not build on FreeBSD
https://bugs.freedesktop.org/show_bug.cgi?id=91826 Albert Freeman changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] texstore byteswap allocation bug
Hey,
On Tue, 2015-08-18 at 12:52 +1000, Dave Airlie wrote:
> Hey,
>
> while running CTS under valgrind I got to see a lot of
>
> ==32256== Invalid read of size 2
> ==32256==at 0x5B53F07: convert_ushort (format_utils.c:1155)
> ==32256==by 0x5B8523A: _mesa_swizzle_and_convert (format_utils.c:1453)
> ==32256==by 0x5B11151: _mesa_format_convert (format_utils.c:354)
> ==32256==by 0x5C07054: texstore_rgba (texstore.c:806)
> ==32256==by 0x5C073C8: _mesa_texstore (texstore.c:930)
> ==32256==by 0x5C078B9: store_texsubimage (texstore.c:1068)
> ==32256==by 0x5C07AC5: _mesa_store_texsubimage (texstore.c:1132)
> ==32256==by 0x5C9A05F: st_TexSubImage (st_cb_texture.c:856)
> ==32256==by 0x5C9A196: st_TexImage (st_cb_texture.c:880)
> ==32256==by 0x5BF1BCC: teximage (teximage.c:3387)
> ==32256==by 0x5BF1D67: _mesa_TexImage2D (teximage.c:3426)
> ==32256==by 0x4CDCA15: glTexImage2D (glapi_mapi_tmp.h:2926)
> ==32256== Address 0xa2b188a is 0 bytes after a block of size 42 alloc'd
> ==32256==at 0x4A06C50: malloc (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==32256==by 0x5C06D97: texstore_rgba (texstore.c:734)
> ==32256==by 0x5C073C8: _mesa_texstore (texstore.c:930)
> ==32256==by 0x5C078B9: store_texsubimage (texstore.c:1068)
> ==32256==by 0x5C07AC5: _mesa_store_texsubimage (texstore.c:1132)
> ==32256==by 0x5C9A05F: st_TexSubImage (st_cb_texture.c:856)
> ==32256==by 0x5C9A196: st_TexImage (st_cb_texture.c:880)
> ==32256==by 0x5BF1BCC: teximage (teximage.c:3387)
> ==32256==by 0x5BF1D67: _mesa_TexImage2D (teximage.c:3426)
> ==32256==by 0x4CDCA15: glTexImage2D (glapi_mapi_tmp.h:2926)
> ==32256==by 0x151483D: glwTexImage2D (glwImpl.inl:482)
> ==32256==by 0xF1BB0B: packedPixelsPixelRectangleInner
> (GTFTestPackedPixels.c:3666)
> ==32256==
>
> which lead to the malloc for the SwapBytes case, being too small. It
> appears the srcRowStride is worked out later at 16-bytes for a width 7
> ushort format, but the byte swap doesn't allocate enough space,
>
> can you guys take a look and suggest a fix, I'm a bit lost there.
sorry for the late reply, I was on holidays...
If we see a srcRowStride of 16-bytes for a width of 7, I guess there are
some packing options involved. We create the swapped image like this:
if (srcPacking->SwapBytes) {
int bytesPerPixel = _mesa_bytes_per_pixel(srcFormat, srcType);
int swapsPerPixel = bytesPerPixel / swapSize;
int elementCount = srcWidth * srcHeight * srcDepth;
assert(bytesPerPixel % swapSize == 0);
tempImage = malloc(elementCount * bytesPerPixel);
if (!tempImage)
return GL_FALSE;
if (swapSize == 2)
_mesa_swap2_copy(tempImage, (GLushort *) srcAddr,
elementCount * swapsPerPixel);
else
_mesa_swap4_copy(tempImage, (GLuint *) srcAddr,
elementCount * swapsPerPixel);
srcAddr = tempImage;
}
I see that this is not considering the packing options of the original
image when allocating the swapped image (or when swapping its data), so
I guess that is the problem.
After that we will do something like:
srcRowStride =
_mesa_image_row_stride(srcPacking, srcWidth, srcFormat, srcType);
which expects the swapped image to have the same packing options as the
original (i.e. it uses srcPacking).
I think that we need to use _mesa_image_row_stride() to compute the size
of the swapped image and then figure out how to incorporate that into
the las parameter in the calls to _mesa_swap_copy() too.
Dave, do you have any piglit tests or other samples that I can use to
reproduce the problem? (I don't have access to the conformance test
suite).
Iago
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] texstore byteswap allocation bug
On Tue, 2015-09-01 at 12:34 +0200, Iago Toral wrote:
> Hey,
>
> On Tue, 2015-08-18 at 12:52 +1000, Dave Airlie wrote:
> > Hey,
> >
> > while running CTS under valgrind I got to see a lot of
> >
> > ==32256== Invalid read of size 2
> > ==32256==at 0x5B53F07: convert_ushort (format_utils.c:1155)
> > ==32256==by 0x5B8523A: _mesa_swizzle_and_convert (format_utils.c:1453)
> > ==32256==by 0x5B11151: _mesa_format_convert (format_utils.c:354)
> > ==32256==by 0x5C07054: texstore_rgba (texstore.c:806)
> > ==32256==by 0x5C073C8: _mesa_texstore (texstore.c:930)
> > ==32256==by 0x5C078B9: store_texsubimage (texstore.c:1068)
> > ==32256==by 0x5C07AC5: _mesa_store_texsubimage (texstore.c:1132)
> > ==32256==by 0x5C9A05F: st_TexSubImage (st_cb_texture.c:856)
> > ==32256==by 0x5C9A196: st_TexImage (st_cb_texture.c:880)
> > ==32256==by 0x5BF1BCC: teximage (teximage.c:3387)
> > ==32256==by 0x5BF1D67: _mesa_TexImage2D (teximage.c:3426)
> > ==32256==by 0x4CDCA15: glTexImage2D (glapi_mapi_tmp.h:2926)
> > ==32256== Address 0xa2b188a is 0 bytes after a block of size 42 alloc'd
> > ==32256==at 0x4A06C50: malloc (in
> > /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==32256==by 0x5C06D97: texstore_rgba (texstore.c:734)
> > ==32256==by 0x5C073C8: _mesa_texstore (texstore.c:930)
> > ==32256==by 0x5C078B9: store_texsubimage (texstore.c:1068)
> > ==32256==by 0x5C07AC5: _mesa_store_texsubimage (texstore.c:1132)
> > ==32256==by 0x5C9A05F: st_TexSubImage (st_cb_texture.c:856)
> > ==32256==by 0x5C9A196: st_TexImage (st_cb_texture.c:880)
> > ==32256==by 0x5BF1BCC: teximage (teximage.c:3387)
> > ==32256==by 0x5BF1D67: _mesa_TexImage2D (teximage.c:3426)
> > ==32256==by 0x4CDCA15: glTexImage2D (glapi_mapi_tmp.h:2926)
> > ==32256==by 0x151483D: glwTexImage2D (glwImpl.inl:482)
> > ==32256==by 0xF1BB0B: packedPixelsPixelRectangleInner
> > (GTFTestPackedPixels.c:3666)
> > ==32256==
> >
> > which lead to the malloc for the SwapBytes case, being too small. It
> > appears the srcRowStride is worked out later at 16-bytes for a width 7
> > ushort format, but the byte swap doesn't allocate enough space,
> >
> > can you guys take a look and suggest a fix, I'm a bit lost there.
>
> sorry for the late reply, I was on holidays...
>
> If we see a srcRowStride of 16-bytes for a width of 7, I guess there are
> some packing options involved. We create the swapped image like this:
>
> if (srcPacking->SwapBytes) {
>int bytesPerPixel = _mesa_bytes_per_pixel(srcFormat, srcType);
>int swapsPerPixel = bytesPerPixel / swapSize;
>int elementCount = srcWidth * srcHeight * srcDepth;
>assert(bytesPerPixel % swapSize == 0);
>tempImage = malloc(elementCount * bytesPerPixel);
>if (!tempImage)
> return GL_FALSE;
>if (swapSize == 2)
> _mesa_swap2_copy(tempImage, (GLushort *) srcAddr,
>elementCount * swapsPerPixel);
>else
> _mesa_swap4_copy(tempImage, (GLuint *) srcAddr,
>elementCount * swapsPerPixel);
>srcAddr = tempImage;
> }
>
> I see that this is not considering the packing options of the original
> image when allocating the swapped image (or when swapping its data), so
> I guess that is the problem.
>
> After that we will do something like:
>
> srcRowStride =
>_mesa_image_row_stride(srcPacking, srcWidth, srcFormat, srcType);
>
> which expects the swapped image to have the same packing options as the
> original (i.e. it uses srcPacking).
>
> I think that we need to use _mesa_image_row_stride() to compute the size
> of the swapped image and then figure out how to incorporate that into
> the las parameter in the calls to _mesa_swap_copy() too.
>
> Dave, do you have any piglit tests or other samples that I can use to
> reproduce the problem? (I don't have access to the conformance test
> suite).
Or maybe an apitrace of a test reproducing the issue...
Iago
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] texstore byteswap allocation bug
On 1 September 2015 at 20:34, Iago Toral wrote:
> Hey,
>
> On Tue, 2015-08-18 at 12:52 +1000, Dave Airlie wrote:
>> Hey,
>>
>> while running CTS under valgrind I got to see a lot of
>>
>> ==32256== Invalid read of size 2
>> ==32256==at 0x5B53F07: convert_ushort (format_utils.c:1155)
>> ==32256==by 0x5B8523A: _mesa_swizzle_and_convert (format_utils.c:1453)
>> ==32256==by 0x5B11151: _mesa_format_convert (format_utils.c:354)
>> ==32256==by 0x5C07054: texstore_rgba (texstore.c:806)
>> ==32256==by 0x5C073C8: _mesa_texstore (texstore.c:930)
>> ==32256==by 0x5C078B9: store_texsubimage (texstore.c:1068)
>> ==32256==by 0x5C07AC5: _mesa_store_texsubimage (texstore.c:1132)
>> ==32256==by 0x5C9A05F: st_TexSubImage (st_cb_texture.c:856)
>> ==32256==by 0x5C9A196: st_TexImage (st_cb_texture.c:880)
>> ==32256==by 0x5BF1BCC: teximage (teximage.c:3387)
>> ==32256==by 0x5BF1D67: _mesa_TexImage2D (teximage.c:3426)
>> ==32256==by 0x4CDCA15: glTexImage2D (glapi_mapi_tmp.h:2926)
>> ==32256== Address 0xa2b188a is 0 bytes after a block of size 42 alloc'd
>> ==32256==at 0x4A06C50: malloc (in
>> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==32256==by 0x5C06D97: texstore_rgba (texstore.c:734)
>> ==32256==by 0x5C073C8: _mesa_texstore (texstore.c:930)
>> ==32256==by 0x5C078B9: store_texsubimage (texstore.c:1068)
>> ==32256==by 0x5C07AC5: _mesa_store_texsubimage (texstore.c:1132)
>> ==32256==by 0x5C9A05F: st_TexSubImage (st_cb_texture.c:856)
>> ==32256==by 0x5C9A196: st_TexImage (st_cb_texture.c:880)
>> ==32256==by 0x5BF1BCC: teximage (teximage.c:3387)
>> ==32256==by 0x5BF1D67: _mesa_TexImage2D (teximage.c:3426)
>> ==32256==by 0x4CDCA15: glTexImage2D (glapi_mapi_tmp.h:2926)
>> ==32256==by 0x151483D: glwTexImage2D (glwImpl.inl:482)
>> ==32256==by 0xF1BB0B: packedPixelsPixelRectangleInner
>> (GTFTestPackedPixels.c:3666)
>> ==32256==
>>
>> which lead to the malloc for the SwapBytes case, being too small. It
>> appears the srcRowStride is worked out later at 16-bytes for a width 7
>> ushort format, but the byte swap doesn't allocate enough space,
>>
>> can you guys take a look and suggest a fix, I'm a bit lost there.
>
> sorry for the late reply, I was on holidays...
>
> If we see a srcRowStride of 16-bytes for a width of 7, I guess there are
> some packing options involved. We create the swapped image like this:
>
> if (srcPacking->SwapBytes) {
>int bytesPerPixel = _mesa_bytes_per_pixel(srcFormat, srcType);
>int swapsPerPixel = bytesPerPixel / swapSize;
>int elementCount = srcWidth * srcHeight * srcDepth;
>assert(bytesPerPixel % swapSize == 0);
>tempImage = malloc(elementCount * bytesPerPixel);
>if (!tempImage)
> return GL_FALSE;
>if (swapSize == 2)
> _mesa_swap2_copy(tempImage, (GLushort *) srcAddr,
>elementCount * swapsPerPixel);
>else
> _mesa_swap4_copy(tempImage, (GLuint *) srcAddr,
>elementCount * swapsPerPixel);
>srcAddr = tempImage;
> }
>
> I see that this is not considering the packing options of the original
> image when allocating the swapped image (or when swapping its data), so
> I guess that is the problem.
>
> After that we will do something like:
>
> srcRowStride =
>_mesa_image_row_stride(srcPacking, srcWidth, srcFormat, srcType);
>
> which expects the swapped image to have the same packing options as the
> original (i.e. it uses srcPacking).
>
> I think that we need to use _mesa_image_row_stride() to compute the size
> of the swapped image and then figure out how to incorporate that into
> the las parameter in the calls to _mesa_swap_copy() too.
>
> Dave, do you have any piglit tests or other samples that I can use to
> reproduce the problem? (I don't have access to the conformance test
> suite).
Hi Iago,
I've posted some follow up patches to fix this behaviour.
Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] mesa: return initial value for VALIDATE_STATUS if pipe not bound
From OpenGL 4.5 Core spec (7.13): "If pipeline is a name that has been generated (without subsequent deletion) by GenProgramPipelines, but refers to a program pipeline object that has not been previously bound, the GL first creates a new state vector in the same manner as when BindProgramPipeline creates a new program pipeline object." I interpret this as "If GetProgramPipelineiv gets called without a bound (but valid) pipeline object, the state should reflect initial state of a new pipeline object." This is also expected behaviour by ES31-CTS.sepshaderobjs.PipelineApi conformance test. Signed-off-by: Tapani Pälli --- src/mesa/main/pipelineobj.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c index 07acbf1..c2e1d29 100644 --- a/src/mesa/main/pipelineobj.c +++ b/src/mesa/main/pipelineobj.c @@ -614,7 +614,8 @@ _mesa_GetProgramPipelineiv(GLuint pipeline, GLenum pname, GLint *params) *params = pipe->InfoLog ? strlen(pipe->InfoLog) + 1 : 0; return; case GL_VALIDATE_STATUS: - *params = pipe->Validated; + /* If pipeline is not bound, return initial value 0. */ + *params = (ctx->_Shader->Name != pipe->Name) ? 0 : pipe->Validated; return; case GL_VERTEX_SHADER: *params = pipe->CurrentProgram[MESA_SHADER_VERTEX] -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] mesa: return initial value for PROGRAM_SEPARABLE when not linked
I haven't found clear spec evidence of this behaviour but this is expected by a conformance test that changes the value with glProgramParameteri but does not link the program. Test says: "The query for PROGRAM_SEPARABLE must query latched state. In other words, the state of the binary after it was linked. So in the tests below, the queries should return the default state GL_FALSE since the program has no linked binary." Signed-off-by: Tapani Pälli --- src/mesa/main/shaderapi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c index 0e0e0d6..fb82543 100644 --- a/src/mesa/main/shaderapi.c +++ b/src/mesa/main/shaderapi.c @@ -773,7 +773,8 @@ get_programiv(struct gl_context *ctx, GLuint program, GLenum pname, return; } case GL_PROGRAM_SEPARABLE: - *params = shProg->SeparateShader; + /* If the program has not been linked, return initial value 0. */ + *params = (shProg->LinkStatus == GL_FALSE) ? 0 : shProg->SeparateShader; return; /* ARB_tessellation_shader */ -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] mesa: enable query of PROGRAM_PIPELINE_BINDING for ES 3.1
Specified in OpenGL ES 3.1 spec, Table 23.32: Program Object State. Signed-off-by: Tapani Pälli --- src/mesa/main/get_hash_params.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py index dc5ba6f..e2e3d0d 100644 --- a/src/mesa/main/get_hash_params.py +++ b/src/mesa/main/get_hash_params.py @@ -458,6 +458,9 @@ descriptor=[ # GL_ARB_explicit_uniform_location / GLES 3.1 [ "MAX_UNIFORM_LOCATIONS", "CONTEXT_INT(Const.MaxUserAssignableUniformLocations), extra_ARB_explicit_uniform_location" ], + +# GL_ARB_separate_shader_objects / GLES 3.1 + [ "PROGRAM_PIPELINE_BINDING", "LOC_CUSTOM, TYPE_INT, GL_PROGRAM_PIPELINE_BINDING, NO_EXTRA" ], ]}, # Enums in OpenGL Core profile and ES 3.1 @@ -799,9 +802,6 @@ descriptor=[ # GL_ARB_texture_gather [ "MAX_PROGRAM_TEXTURE_GATHER_COMPONENTS_ARB", "CONTEXT_INT(Const.MaxProgramTextureGatherComponents), extra_ARB_texture_gather"], -# GL_ARB_separate_shader_objects - [ "PROGRAM_PIPELINE_BINDING", "LOC_CUSTOM, TYPE_INT, GL_PROGRAM_PIPELINE_BINDING, NO_EXTRA" ], - # GL_ARB_shader_atomic_counters [ "MAX_GEOMETRY_ATOMIC_COUNTER_BUFFERS", "CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxAtomicBuffers), extra_ARB_shader_atomic_counters_and_geometry_shader" ], [ "MAX_GEOMETRY_ATOMIC_COUNTERS", "CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxAtomicCounters), extra_ARB_shader_atomic_counters_and_geometry_shader" ], -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: change 'SHADER_SUBST' facility to work with env variables
On 08/31/2015 05:56 PM, Brian Paul wrote:
Looks good. Just some minor nitpicks below.
Reviewed-by: Brian Paul
Thanks! I have one comment below ..
On 08/31/2015 01:23 AM, Tapani Pälli wrote:
Patch modifies existing shader source and replace functionality to work
with environment variables rather than enable dumping on compile time.
Also instead of _mesa_str_checksum, _mesa_sha1_compute is used to avoid
collisions.
Functionality is controlled via 2 environment variables:
s/2/two/
MESA_SHADER_DUMP_PATH - path where shader sources are dumped
MESA_SHADER_READ_PATH - path where replacement shaders are read
Signed-off-by: Tapani Pälli
Suggested-by: Eero Tamminen
---
docs/shading.html | 9 +++
src/mesa/main/shaderapi.c | 136
--
2 files changed, 105 insertions(+), 40 deletions(-)
diff --git a/docs/shading.html b/docs/shading.html
index 77a0ee4..784329e 100644
--- a/docs/shading.html
+++ b/docs/shading.html
@@ -63,6 +63,15 @@ execution. These are generally used for debugging.
Example: export MESA_GLSL=dump,nopt
+
+Shaders can be dumped and replaced on runtime for debugging purposes.
+This is controlled via following environment variables:
+
+MESA_SHADER_DUMP_PATH - path where shader sources are dumped
+MESA_SHADER_READ_PATH - path where replacement shaders are
read
Do these directories need to be different to prevent collisions between
the dumped shaders and the replacement shaders?
Yes, I thought a bit of solutions to work only in one dir but I think
having 2 makes everything more clear and here user can be only one
overwriting edited contents. I could add some text here : "These paths
should be separate to avoid overwriting modified contents." (?)
+
+Note, path set must exist before running for dumping or replacing to
work.
+
GLSL Version
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 0e0e0d6..d3abfc9 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -53,15 +53,13 @@
#include "program/prog_parameter.h"
#include "util/ralloc.h"
#include "util/hash_table.h"
+#include "util/mesa-sha1.h"
#include
#include "../glsl/glsl_parser_extras.h"
#include "../glsl/ir.h"
#include "../glsl/ir_uniform.h"
#include "../glsl/program.h"
-/** Define this to enable shader substitution (see below) */
-#define SHADER_SUBST 0
-
/**
* Return mask of GLSL_x flags by examining the MESA_GLSL env var.
@@ -1512,24 +1510,101 @@ _mesa_LinkProgram(GLhandleARB programObj)
link_program(ctx, programObj);
}
+/**
+ * Generate a SHA-1 hash value string for given source string.
+ */
+static void
+generate_sha1(const char *source, char sha_str[64])
+{
+ unsigned char sha[20];
+ _mesa_sha1_compute(source, strlen(source), sha);
+ _mesa_sha1_format(sha_str, sha);
+}
+
+/**
+ * Construct a full path for shader replacement functionality using
+ * following format:
+ *
+ * /_.glsl
+ */
+static void
+construct_name(const gl_shader_stage stage, const char *source,
+ const char *path, char *name, unsigned length)
+{
+ char sha[64];
+ static const char *types[] = {
+ "VS", "TC", "TE", "GS", "FS", "CS",
+ };
+ generate_sha1(source, sha);
+ _mesa_snprintf(name, length, "%s/%s_%s.glsl", path, types[stage],
+ sha);
+}
+
+/**
+ * Write given shader source to a file in MESA_SHADER_DUMP_PATH.
+ */
+static void
+dump_shader(const gl_shader_stage stage, const char *source)
+{
+ char name[PATH_MAX];
+ static bool path_exists = true;
+ char *dump_path;
+ FILE *f;
+
+ if (!path_exists)
+ return NULL;
+
+ dump_path = getenv("MESA_SHADER_DUMP_PATH");
+
Could remove that empty line.
+ if (!dump_path) {
+ path_exists = false;
+ return NULL;
+ }
+
+ construct_name(stage, source, dump_path, name, PATH_MAX);
+
+ f = fopen(name, "w");
+ if (f) {
+ fputs(source, f);
+ fclose(f);
+ } else {
+ GET_CURRENT_CONTEXT(ctx);
+ _mesa_warning(ctx, "could not open %s for dumping shader", name);
+ }
+}
/**
* Read shader source code from a file.
* Useful for debugging to override an app's shader.
*/
static GLcharARB *
-read_shader(const char *fname)
+read_shader(const gl_shader_stage stage, const char *source)
{
- int shader_size = 0;
- FILE *f = fopen(fname, "r");
- GLcharARB *buffer, *shader;
- int len;
+ char name[PATH_MAX];
+ char *read_path;
+ static bool path_exists = true;
+ int len, shader_size = 0;
+ GLcharARB *buffer;
+ FILE *f;
- if (!f) {
+ if (!path_exists)
+ return NULL;
+
+ read_path = getenv("MESA_SHADER_READ_PATH");
+
+ if (!read_path) {
+ path_exists = false;
return NULL;
}
+ construct_name(stage, source, read_path, name, PATH_MAX);
+
+ f = fopen(name, "r");
+
Could remove that empty line.
+ if (!f)
+ return NULL;
+
/* allocate enough room for the entire shader */
fseek(f, 0, SEEK_END);
Re: [Mesa-dev] [PATCH] meta: Compute correct buffer size with SkipRows/SkipPixels
Good catch and it seems like a nice way to fix it. Reviewed-by: Neil Roberts I wonder if it might be worth avoiding copying the padding and pack the rows more tightly in the temporary buffer. Ie, we would allocate a buffer of align(width*cpp)*height*depth and copy the rows in one at a time instead of memcpying the whole buffer. As it stands for example if you are using the stride to make a 16x16 texture of a subregion of a 1024x1024 buffer we will pointlessly make a temporary buffer of 1024x16. I'm not saying we should hold up this fix for that though. Regards, - Neil Chris Wilson writes: > From: Jason Ekstrand > > If the user is specifying a subregion of a buffer using SKIP_ROWS and > SKIP_PIXELS, we must compute the buffer size carefully as the end of the > last row may be much shorter than stride*image_height*depth. The current > code tries to memcpy from beyond the end of the user data, for example > causing: > > ==28136== Invalid read of size 8 > ==28136==at 0x4C2D94E: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:915) > ==28136==by 0xB4ADFE3: brw_bo_write (brw_batch.c:1856) > ==28136==by 0xB5B3531: brw_buffer_data (intel_buffer_objects.c:208) > ==28136==by 0xB0F6275: _mesa_buffer_data (bufferobj.c:1600) > ==28136==by 0xB0F6346: _mesa_BufferData (bufferobj.c:1631) > ==28136==by 0xB37A1EE: create_texture_for_pbo (meta_tex_subimage.c:103) > ==28136==by 0xB37A467: _mesa_meta_pbo_TexSubImage > (meta_tex_subimage.c:176) > ==28136==by 0xB5C8D61: intelTexSubImage (intel_tex_subimage.c:195) > ==28136==by 0xB254AB4: _mesa_texture_sub_image (teximage.c:3654) > ==28136==by 0xB254C9F: texsubimage (teximage.c:3712) > ==28136==by 0xB2550E9: _mesa_TexSubImage2D (teximage.c:3853) > ==28136==by 0x401CA0: UploadTexSubImage2D (teximage.c:171) > ==28136== Address 0xd8bfbe0 is 0 bytes after a block of size 1,024 alloc'd > ==28136==at 0x4C28C20: malloc (vg_replace_malloc.c:296) > ==28136==by 0x402014: PerfDraw (teximage.c:270) > ==28136==by 0x402648: Draw (glmain.c:182) > ==28136==by 0x8385E63: ??? (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136==by 0x83896C8: fgEnumWindows (in > /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136==by 0x838641C: glutMainLoopEvent (in > /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136==by 0x8386C1C: glutMainLoop (in > /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136==by 0x4019C1: main (glmain.c:262) > ==28136== > ==28136== Invalid read of size 8 > ==28136==at 0x4C2D940: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:915) > ==28136==by 0xB4ADFE3: brw_bo_write (brw_batch.c:1856) > ==28136==by 0xB5B3531: brw_buffer_data (intel_buffer_objects.c:208) > ==28136==by 0xB0F6275: _mesa_buffer_data (bufferobj.c:1600) > ==28136==by 0xB0F6346: _mesa_BufferData (bufferobj.c:1631) > ==28136==by 0xB37A1EE: create_texture_for_pbo (meta_tex_subimage.c:103) > ==28136==by 0xB37A467: _mesa_meta_pbo_TexSubImage > (meta_tex_subimage.c:176) > ==28136==by 0xB5C8D61: intelTexSubImage (intel_tex_subimage.c:195) > ==28136==by 0xB254AB4: _mesa_texture_sub_image (teximage.c:3654) > ==28136==by 0xB254C9F: texsubimage (teximage.c:3712) > ==28136==by 0xB2550E9: _mesa_TexSubImage2D (teximage.c:3853) > ==28136==by 0x401CA0: UploadTexSubImage2D (teximage.c:171) > ==28136== Address 0xd8bfbe8 is 8 bytes after a block of size 1,024 alloc'd > ==28136==at 0x4C28C20: malloc (vg_replace_malloc.c:296) > ==28136==by 0x402014: PerfDraw (teximage.c:270) > ==28136==by 0x402648: Draw (glmain.c:182) > ==28136==by 0x8385E63: ??? (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136==by 0x83896C8: fgEnumWindows (in > /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136==by 0x838641C: glutMainLoopEvent (in > /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136==by 0x8386C1C: glutMainLoop (in > /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136==by 0x4019C1: main (glmain.c:262) > ==28136== > > Fixes: 7f396189f073d626c5f7a2c232dac92b65f5a23f > Cc: Jason Ekstrand > Cc: Neil Roberts > --- > src/mesa/drivers/common/meta_tex_subimage.c | 35 > + > 1 file changed, 21 insertions(+), 14 deletions(-) > > diff --git a/src/mesa/drivers/common/meta_tex_subimage.c > b/src/mesa/drivers/common/meta_tex_subimage.c > index 16d8f5d..e2351c6 100644 > --- a/src/mesa/drivers/common/meta_tex_subimage.c > +++ b/src/mesa/drivers/common/meta_tex_subimage.c > @@ -46,8 +46,9 @@ > #include "varray.h" > > static struct gl_texture_image * > -create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, > - GLenum pbo_target, int width, int height, > +create_texture_for_pbo(struct gl_context *ctx, > + bool create_pbo, GLenum pbo_target, > + int dims, int width, int height, int depth, > GLenum format, GLenum type, const void *pixels, >
Re: [Mesa-dev] [PATCH] meta: Compute correct buffer size with SkipRows/SkipPixels
On Tue, Sep 01, 2015 at 01:09:33PM +0100, Neil Roberts wrote: > Good catch and it seems like a nice way to fix it. > > Reviewed-by: Neil Roberts > > I wonder if it might be worth avoiding copying the padding and pack the > rows more tightly in the temporary buffer. Ie, we would allocate a > buffer of align(width*cpp)*height*depth and copy the rows in one at a > time instead of memcpying the whole buffer. As it stands for example if > you are using the stride to make a 16x16 texture of a subregion of a > 1024x1024 buffer we will pointlessly make a temporary buffer of 1024x16. > I'm not saying we should hold up this fix for that though. I've been half wondering about that as well. There seems to be a deficit of good texture up/downloading benchmarks, that both stress all pathways and reflect real world usage. For the latter, I wonder if apitrace could be coaxed into service? (You need realistic GPU usage in addition to texture transfers to answer questions such as blit vs stall.) As for the former, we could just repurpose some of the API coverage tests in piglit to serve as comprehensive micro-benchmarks. I have a v2 of this patch because piglit tells me I can't just drop full_height on the floor - as the teximage we create is meant to be width x full_height x 1. Oops. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] No VAAPI driver hardlinks?
On 31 August 2015 at 13:32, Emil Velikov wrote: > On 30 August 2015 at 00:21, Matt Turner wrote: >> Hi Emil, >> >> src/gallium/targets/vdpau has a block that installs per-driver >> hardlinks, but src/gallium/targets/va does not (presumably because it >> was added later), which leads to: >> >> https://bugs.gentoo.org/549564 >> >> Presumably it's mostly a matter of copy-n-paste? >> > I believe I opted against the hardlinks as only r600 was supported > initially, not 100% sure. Considering that radeonsi works (plus > possible nouveau support on the horizon) I don't have any objections > if we add the hunk for the vaapi targets. Will send a patch in a bit. > >> Also, it appears that there's a minor problem with that block as well: >> >> https://bugs.gentoo.org/545230 >> > Quite odd that one will enable vdpau if they don't build any of > r300/r600/radeonsi/nouveau. Either way a fix will be coming shortly, > covering all the targets. > Grr... this is being more annoying than expected. The core issue is that as soon as we end up in the makefile, the directory is created by automake. The latter does not keep track if it existed before we started or not. Namely I have a patch that will remove the directory if empty, although we shouldn't do that if it was there in the first place. Does that sound like a reasonable thing to do, or should we consider this a "feature" :-) Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: change 'SHADER_SUBST' facility to work with env variables
On 09/01/2015 06:04 AM, Tapani Pälli wrote:
On 08/31/2015 05:56 PM, Brian Paul wrote:
Looks good. Just some minor nitpicks below.
Reviewed-by: Brian Paul
Thanks! I have one comment below ..
On 08/31/2015 01:23 AM, Tapani Pälli wrote:
Patch modifies existing shader source and replace functionality to work
with environment variables rather than enable dumping on compile time.
Also instead of _mesa_str_checksum, _mesa_sha1_compute is used to avoid
collisions.
Functionality is controlled via 2 environment variables:
s/2/two/
MESA_SHADER_DUMP_PATH - path where shader sources are dumped
MESA_SHADER_READ_PATH - path where replacement shaders are read
Signed-off-by: Tapani Pälli
Suggested-by: Eero Tamminen
---
docs/shading.html | 9 +++
src/mesa/main/shaderapi.c | 136
--
2 files changed, 105 insertions(+), 40 deletions(-)
diff --git a/docs/shading.html b/docs/shading.html
index 77a0ee4..784329e 100644
--- a/docs/shading.html
+++ b/docs/shading.html
@@ -63,6 +63,15 @@ execution. These are generally used for debugging.
Example: export MESA_GLSL=dump,nopt
+
+Shaders can be dumped and replaced on runtime for debugging purposes.
+This is controlled via following environment variables:
+
+MESA_SHADER_DUMP_PATH - path where shader sources are dumped
+MESA_SHADER_READ_PATH - path where replacement shaders are
read
Do these directories need to be different to prevent collisions between
the dumped shaders and the replacement shaders?
Yes, I thought a bit of solutions to work only in one dir but I think
having 2 makes everything more clear and here user can be only one
overwriting edited contents. I could add some text here : "These paths
should be separate to avoid overwriting modified contents." (?)
How about "When both are set, these paths should be different so the
dumped shaders do not clobber the replacement shaders."
-Brian
+
+Note, path set must exist before running for dumping or replacing to
work.
+
GLSL Version
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 0e0e0d6..d3abfc9 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -53,15 +53,13 @@
#include "program/prog_parameter.h"
#include "util/ralloc.h"
#include "util/hash_table.h"
+#include "util/mesa-sha1.h"
#include
#include "../glsl/glsl_parser_extras.h"
#include "../glsl/ir.h"
#include "../glsl/ir_uniform.h"
#include "../glsl/program.h"
-/** Define this to enable shader substitution (see below) */
-#define SHADER_SUBST 0
-
/**
* Return mask of GLSL_x flags by examining the MESA_GLSL env var.
@@ -1512,24 +1510,101 @@ _mesa_LinkProgram(GLhandleARB programObj)
link_program(ctx, programObj);
}
+/**
+ * Generate a SHA-1 hash value string for given source string.
+ */
+static void
+generate_sha1(const char *source, char sha_str[64])
+{
+ unsigned char sha[20];
+ _mesa_sha1_compute(source, strlen(source), sha);
+ _mesa_sha1_format(sha_str, sha);
+}
+
+/**
+ * Construct a full path for shader replacement functionality using
+ * following format:
+ *
+ * /_.glsl
+ */
+static void
+construct_name(const gl_shader_stage stage, const char *source,
+ const char *path, char *name, unsigned length)
+{
+ char sha[64];
+ static const char *types[] = {
+ "VS", "TC", "TE", "GS", "FS", "CS",
+ };
+ generate_sha1(source, sha);
+ _mesa_snprintf(name, length, "%s/%s_%s.glsl", path, types[stage],
+ sha);
+}
+
+/**
+ * Write given shader source to a file in MESA_SHADER_DUMP_PATH.
+ */
+static void
+dump_shader(const gl_shader_stage stage, const char *source)
+{
+ char name[PATH_MAX];
+ static bool path_exists = true;
+ char *dump_path;
+ FILE *f;
+
+ if (!path_exists)
+ return NULL;
+
+ dump_path = getenv("MESA_SHADER_DUMP_PATH");
+
Could remove that empty line.
+ if (!dump_path) {
+ path_exists = false;
+ return NULL;
+ }
+
+ construct_name(stage, source, dump_path, name, PATH_MAX);
+
+ f = fopen(name, "w");
+ if (f) {
+ fputs(source, f);
+ fclose(f);
+ } else {
+ GET_CURRENT_CONTEXT(ctx);
+ _mesa_warning(ctx, "could not open %s for dumping shader", name);
+ }
+}
/**
* Read shader source code from a file.
* Useful for debugging to override an app's shader.
*/
static GLcharARB *
-read_shader(const char *fname)
+read_shader(const gl_shader_stage stage, const char *source)
{
- int shader_size = 0;
- FILE *f = fopen(fname, "r");
- GLcharARB *buffer, *shader;
- int len;
+ char name[PATH_MAX];
+ char *read_path;
+ static bool path_exists = true;
+ int len, shader_size = 0;
+ GLcharARB *buffer;
+ FILE *f;
- if (!f) {
+ if (!path_exists)
+ return NULL;
+
+ read_path = getenv("MESA_SHADER_READ_PATH");
+
+ if (!read_path) {
+ path_exists = false;
return NULL;
}
+ construct_name(stage, source, read_path, name, PATH_MAX);
Re: [Mesa-dev] [PATCH 00/12] bmake inspired fixes
On 21 August 2015 at 19:09, Emil Velikov wrote: > On 03/08/15 19:09, Emil Velikov wrote: >> On 3 August 2015 at 17:17, Matt Turner wrote: >>> On Fri, Jul 17, 2015 at 11:53 AM, Emil Velikov >>> wrote: On 17 July 2015 at 19:09, Matt Turner wrote: > On Fri, Jul 17, 2015 at 10:29 AM, Emil Velikov > wrote: >> Hello all, >> >> A few days ago I realised that BSD make (bmake) is available in the >> Archlinux repos, so I decided to give it a try for drm & mesa. >> >> While the former was working (minus a small patch) mesa is not so lucky. >> >> This series attempts to remove the GNU make idioms, with the first two >> being the base essential for a successful build from tarball. > > ... why should we care about non-GNU make? GNU make has nice features > that we want to use and we use them. I don't see the benefit. > A few reasons: - It will allow the OpenBSD people to use upstream mesa and devote that time to something more useful ? >>> >>> Mesa builds on OpenBSD already, as far as I know. The build system >>> isn't holding back contributions. >>> >> They use an in-house bmake compatible system rather. So as they hit a >> bug, it's hard to establish if it's due to their build or not. That, >> plus the serious rework they need to do in their build, contributes as >> to why they're not updating mesa as frequently. >> Would be great to spare them those obstacles, even if they choose to >> be slightly different ;-) >> >>> I still don't follow how making the build system compatible with >>> non-GNU make is beneficial. >>> >> Let try this from another angle. Even if there is zero benefit, do you >> foresee any issues with making it compatible ? Afaics it won't make >> anyone's job harder - I/Jonathan will send a quick every so often and >> things will just work for everyone. Or maybe there is some subtlety >> that I'm missing ? >> >> As mentioned before - there seems to be only one pattern "at fault", >> plus it's been addressed with the series. >> - Mostly a single pattern/issue/thinko seems to be at fault. - The rules already look a bit shaky :-) >>> >>> I don't understand what these mean. >> Imho a handful of the Makefiles in src/mapi src/mesa/ are inconsistent >> (and confusing) comparing to their dri/glx/egl/gallium counterparts. >> >> The lex/bison/python rules being a good example. With these we provide >> explicit info (expand $<) and provide a more consistent look. If they >> look harder to read/grasp/etc. just say so and I'll update things >> accordingly. >> > Hi Matt, > > Please, state your technical conserns, elaborating a bit on each one, so > that I can try and address them. > > I'm still uncertain why you are unhappy with the series - is it because > it starts with "bmake" :P. replacing "$<" with "foo.py" can cause > confusion/issues in the long term, you are planning on introducing some > other GNUmake specific constructs or something else perhaps ? > Humble reminder. If you'd like some objective justification why these patches make things better, please give me some merits that I can check against. Alternatively I'll push these within a few days. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 2/2] clover: Use threadsafe wrappers for pipe_context v2
Hi guys On 11 July 2015 at 11:47, Francisco Jerez wrote: > Tom Stellard writes: > >> Events can be added to an OpenCL command queue concurrently from multiple >> threads, but pipe_context bjects are not threadsafe. The threadsafe >> wrappers protect all pipe_context function calls with a mutex, so we >> can safely use them with multiple threads. >> >> v2: >> - Don't use wrapper for pipe_screen. >> >> CC: 10.6 > > Thanks, this patch is: > Reviewed-by: Francisco Jerez > This hasn't landed in master yet. Are they any issues with it ? Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V3] glsl: fix atomic buffer index for bindings other than 0
On 29 July 2015 at 13:46, Timothy Arceri wrote: > On Wed, 2015-07-29 at 09:57 +0200, Iago Toral wrote: >> On Sun, 2015-07-26 at 18:35 +1000, Timothy Arceri wrote: >> > Since commit c0cd5b var->data.binding was being used as a replacement >> > for atomic buffer index, but they don't have to be the same value they >> > just happen to end up the same when binding is 0. >> > >> > Now we store atomic buffer index in the unused var->data.index >> > to avoid the extra memory of putting back the atmoic buffer index field. >> >> Could this be a bit too restrictive? var->data.index has only a single >> bit of storage, so this would limit the number of buffers we can index >> to 2. > > Your right I wasn't paying enough attention, the nir struct doesn't place the > same limits on index (although maybe it should) and I didn't notice it in the > glsl ir. > > I have a new solution on the way as part on V3 of my AoA work, however its not > really suitable for stable. > > If we want this fix in stable maybe putting back the atomic_index struct > member is the best solution after all. > I guess we can/should drop this from the queue, or is it something still worthy for stable ? If so, can anyone let me know of the requirements (commit name and/or sha should be great). Thanks, Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 16/18] i965: Prevent coordinate overflow in intel_emit_linear_blit
On 1 August 2015 at 17:57, Emil Velikov wrote:
> Hello all,
>
> On 20 July 2015 at 18:08, Anuj Phogat wrote:
>> On Sat, Jul 18, 2015 at 1:24 AM, Chris Wilson
>> wrote:
>>> On Fri, Jul 17, 2015 at 05:12:54PM -0700, Anuj Phogat wrote:
On Mon, Jul 6, 2015 at 3:33 AM, Chris Wilson
wrote:
> + do {
> + /* The pitch given to the GPU must be DWORD aligned, and
> + * we want width to match pitch. Max width is (1 << 15 - 1),
> + * rounding that down to the nearest DWORD is 1 << 15 - 4
> + */
> + pitch = ROUND_DOWN_TO(MIN2(size, (1 << 15) - 64), 4);
I understand why you are subtracting 64 in above statement, it'll
be nice to update above comment explaining the reason.
>>>
>>> We use the pitch to set the copy width, so the maximum x coordinate
>>> becomes src_x + pitch. Since src_x has a maximum value of 63, we want to
>>> make sure that pitch is less than 32627-63. Simplified above.
>>>
> + height = (size < pitch || pitch == 0) ? 1 : size / pitch;
>>> ...
> + pitch *= height;
> + if (size <= pitch)
I think size < pitch will never be true. How about:
assert(size < pitch);
>>>
>>> For a single row copy, size can be less than pitch.
>> right.
>>
>> Reviewed-by: Anuj Phogat
> It doesn't seem that the patch has landed in master despite the review
> from Anuj. Is it missing something or did it fell through the cracks ?
>
Another humble ping !
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] xa: add xa_surface_from_handle2
Hi all, On 1 August 2015 at 20:40, Thomas Hellstrom wrote: > Hi! > > On 08/01/2015 07:16 PM, Emil Velikov wrote: >> On 22 July 2015 at 00:00, Rob Clark wrote: >>> From: Rob Clark >>> >>> Like xa_surface_from_handle(), but takes a handle type, rather than >>> hard-coding 'shared' handle. This is needed to fix bugs seen with >>> xf86-video-freedreno with xrandr rotation, for example. The root issue >>> is that doing a GEM_OPEN ioctl on a bo that already has a GEM handle >>> associated with the drm_file will result in two unique handles for the >>> same bo. Which causes all sorts of follow-on fail. >>> >>> Cc: "10.5 10.6" >>> Signed-off-by: Rob Clark >>> --- >>> Note: it would be good to get this in stable too, since I have a patch >>> for xf86-video-freedreno which will depend on this. >>> >> Bth, I'm not too excited about having new APIs in the stable branch, >> despite them being trivial as this one. >> Regardless it would be nice to get a pair of eyes looking in this direction. >> >> The patch does exactly what it says on the tin, although I do wonder >> if we should bump XA_TRACKER_VERSION_MINOR ? > > I haven't had time yet to look at the patch itself (I'm on vacation and > back next week), but having XA recognize handle types, in particular > dma-buf fds is, IMO a good thing. > > Regarding adding new interfaces to XA, the XA documentation is pretty > explicit about how this should be handled and recognized, with version > numbers. As long as all XA users follow this, it should be pretty > straightforward also in a stable mesa branch. IIRC, the XA minor must be > bumped in this case, and an out-of-mesa-tree user can't rely on the > symbol being present, since an old libxatracker with the same major > number might be present, but needs to use dlsym() or something similar. > Rob, Thomas do you plan on picking this up and addressing the final bits ? -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] st/readpixels: fix accel path for skipimages.
Reviewed-by: Iago Toral Quiroga
On Tue, 2015-09-01 at 16:41 +1000, Dave Airlie wrote:
> From: Dave Airlie
>
> We don't need to use the 3d image address here as that will
> include SKIP_IMAGES, and we are only blitting a single
> 2D anyways, so just use the 2D path.
>
> This fixes some memory overruns under CTS
> packed_pixels.packed_pixels_pixelstore when PACK_SKIP_IMAGES
> is used.
>
> Signed-off-by: Dave Airlie
> ---
> src/mesa/state_tracker/st_cb_readpixels.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_readpixels.c
> b/src/mesa/state_tracker/st_cb_readpixels.c
> index 6ff6cf6..bb36e69 100644
> --- a/src/mesa/state_tracker/st_cb_readpixels.c
> +++ b/src/mesa/state_tracker/st_cb_readpixels.c
> @@ -238,9 +238,9 @@ st_readpixels(struct gl_context *ctx, GLint x, GLint y,
>GLuint row;
>
>for (row = 0; row < (unsigned) height; row++) {
> - GLvoid *dest = _mesa_image_address3d(pack, pixels,
> + GLvoid *dest = _mesa_image_address2d(pack, pixels,
>width, height, format,
> - type, 0, row, 0);
> + type, row, 0);
> memcpy(dest, map, bytesPerRow);
> map += tex_xfer->stride;
>}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] clover: Properly initialize LLVM targets when linking with component libs
On 8 August 2015 at 12:11, Francisco Jerez wrote:
> Tom Stellard writes:
>
>> Calls to LLVMIntialize* fail when we are linking against individual
>> component libraries rather than one large shared object, because
>> we only include component libraries that are required by the drivers.
>>
>> We need to make sure to only initialize the targets that we need.
>>
>> CC: 10.6
>> ---
>> configure.ac | 4
>> src/gallium/state_trackers/clover/Makefile.am | 3 ++-
>> src/gallium/state_trackers/clover/llvm/invocation.cpp | 17 +
>> 3 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 36197d3..e1a7d7a 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -2040,8 +2040,10 @@ require_egl_drm() {
>> radeon_llvm_check() {
>> if test ${LLVM_VERSION_INT} -lt 307; then
>> amdgpu_llvm_target_name='r600'
>> + CLOVER_CPP_FLAGS="${CLOVER_CPP_FLAGS} -DCLOVER_INIT_R600_TARGET"
>> else
>> amdgpu_llvm_target_name='amdgpu'
>> + CLOVER_CPP_FLAGS="${CLOVER_CPP_FLAGS} -DCLOVER_INIT_AMDGPU_TARGET"
>> fi
>> if test "x$enable_gallium_llvm" != "xyes"; then
>> AC_MSG_ERROR([--enable-gallium-llvm is required when building $1])
>> @@ -2285,6 +2287,8 @@ AC_SUBST([XA_MINOR], $XA_MINOR)
>> AC_SUBST([XA_TINY], $XA_TINY)
>> AC_SUBST([XA_VERSION], "$XA_MAJOR.$XA_MINOR.$XA_TINY")
>>
>> +AC_SUBST([CLOVER_CPP_FLAGS], $CLOVER_CPP_FLAGS)
>> +
>> dnl Restore LDFLAGS and CPPFLAGS
>> LDFLAGS="$_SAVE_LDFLAGS"
>> CPPFLAGS="$_SAVE_CPPFLAGS"
>> diff --git a/src/gallium/state_trackers/clover/Makefile.am
>> b/src/gallium/state_trackers/clover/Makefile.am
>> index fd0ccf8..975b36f 100644
>> --- a/src/gallium/state_trackers/clover/Makefile.am
>> +++ b/src/gallium/state_trackers/clover/Makefile.am
>> @@ -45,7 +45,8 @@ libclllvm_la_CXXFLAGS = \
>> $(DEFINES) \
>> -DLIBCLC_INCLUDEDIR=\"$(LIBCLC_INCLUDEDIR)/\" \
>> -DLIBCLC_LIBEXECDIR=\"$(LIBCLC_LIBEXECDIR)/\" \
>> - -DCLANG_RESOURCE_DIR=\"$(CLANG_RESOURCE_DIR)\"
>> + -DCLANG_RESOURCE_DIR=\"$(CLANG_RESOURCE_DIR)\" \
>> + $(CLOVER_CPP_FLAGS)
>>
>> libclllvm_la_SOURCES = $(LLVM_SOURCES)
>>
>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> index 86859af..361a149 100644
>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> @@ -786,10 +786,19 @@ namespace {
>> init_targets() {
>>static bool targets_initialized = false;
>>if (!targets_initialized) {
>> - LLVMInitializeAllTargets();
>> - LLVMInitializeAllTargetInfos();
>> - LLVMInitializeAllTargetMCs();
>> - LLVMInitializeAllAsmPrinters();
>> +#ifdef CLOVER_INIT_AMDGPU_TARGET
>> + LLVMInitializeAMDGPUTarget();
>> + LLVMInitializeAMDGPUTargetInfo();
>> + LLVMInitializeAMDGPUTargetMC();
>> + LLVMInitializeAMDGPUAsmPrinter();
>> +#endif
>> +
>> +#ifdef CLOVER_INIT_R600_TARGET
>> + LLVMInitializeR600Target();
>> + LLVMInitializeR600TargetInfo();
>> + LLVMInitializeR600TargetMC();
>> + LLVMInitializeR600AsmPrinter();
>> +#endif
>
> Doesn't this feel like a layering violation? Why should clover
> initialize specific LLVM back-ends? And isn't it a build-system bug if,
> say, LLVMInitializeAllTargets() is being pulled in but some symbol it
> depends on isn't?
>
Hi Tom,
Based of Francisco's comment it seems that this patch goes in the bit
bucket. Can you please confirm if that's not the case ?
Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] xa: add xa_surface_from_handle2
On 09/01/2015 03:49 PM, Emil Velikov wrote: > Hi all, > > On 1 August 2015 at 20:40, Thomas Hellstrom wrote: >> Hi! >> >> On 08/01/2015 07:16 PM, Emil Velikov wrote: >>> On 22 July 2015 at 00:00, Rob Clark wrote: From: Rob Clark Like xa_surface_from_handle(), but takes a handle type, rather than hard-coding 'shared' handle. This is needed to fix bugs seen with xf86-video-freedreno with xrandr rotation, for example. The root issue is that doing a GEM_OPEN ioctl on a bo that already has a GEM handle associated with the drm_file will result in two unique handles for the same bo. Which causes all sorts of follow-on fail. Cc: "10.5 10.6" Signed-off-by: Rob Clark --- Note: it would be good to get this in stable too, since I have a patch for xf86-video-freedreno which will depend on this. >>> Bth, I'm not too excited about having new APIs in the stable branch, >>> despite them being trivial as this one. >>> Regardless it would be nice to get a pair of eyes looking in this direction. >>> >>> The patch does exactly what it says on the tin, although I do wonder >>> if we should bump XA_TRACKER_VERSION_MINOR ? >> I haven't had time yet to look at the patch itself (I'm on vacation and >> back next week), but having XA recognize handle types, in particular >> dma-buf fds is, IMO a good thing. >> >> Regarding adding new interfaces to XA, the XA documentation is pretty >> explicit about how this should be handled and recognized, with version >> numbers. As long as all XA users follow this, it should be pretty >> straightforward also in a stable mesa branch. IIRC, the XA minor must be >> bumped in this case, and an out-of-mesa-tree user can't rely on the >> symbol being present, since an old libxatracker with the same major >> number might be present, but needs to use dlsym() or something similar. >> > Rob, Thomas do you plan on picking this up and addressing the final bits ? > > -Emil Hi! we'll need this for dri3 support in vmwgfx but for us there's no need to take it through stable. Rob, are you going for glamor or do you need this for stable? Thanks, Thomas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965: Abort tiled_memcpy path for ReadPixels in case of transfer operations
Hi all On 21 August 2015 at 23:04, Anuj Phogat wrote: > We have a similar check in meta pbo path. > > Cc: > Signed-off-by: Anuj Phogat > --- > src/mesa/drivers/dri/i965/intel_pixel_read.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/intel_pixel_read.c > b/src/mesa/drivers/dri/i965/intel_pixel_read.c > index 3fe506e..55f6852 100644 > --- a/src/mesa/drivers/dri/i965/intel_pixel_read.c > +++ b/src/mesa/drivers/dri/i965/intel_pixel_read.c > @@ -81,6 +81,10 @@ intel_readpixels_tiled_memcpy(struct gl_context * ctx, > if (rb == NULL) >return false; > > + if (_mesa_get_readpixels_transfer_ops(ctx, rb->Format, format, > + type, GL_FALSE)) > + return false; > + > struct intel_renderbuffer *irb = intel_renderbuffer(rb); > int dst_pitch; > Can we get a pair of eyes looking this way (same goes for the rest of the series). They all seem like pretty trivial fixes. Anuj, can you amend the cc tag before pushing to include 10.6 please. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] mesa/readpixels: check strides are equal before skipping conversion
Reviewed-by: Iago Toral Quiroga
On Tue, 2015-09-01 at 16:41 +1000, Dave Airlie wrote:
> From: Dave Airlie
>
> The CTS packed_pixels test checks that readpixels doesn't write
> into the space between rows, however we fail that here unless
> we check the format and stride match.
> This fixes all the core mesa problems with CTS packed_pixels
> tests.
>
> Signed-off-by: Dave Airlie
> ---
> src/mesa/main/readpix.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index 0ef07b5..c57fbac 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -523,7 +523,8 @@ read_rgba_pixels( struct gl_context *ctx,
> * convert to, then we can convert directly into the dst buffer and
> avoid
> * the final conversion/copy from the rgba buffer to the dst buffer.
> */
> - if (dst_format == rgba_format) {
> + if (dst_format == rgba_format &&
> + dst_stride == rgba_stride) {
> need_convert = false;
> rgba = dst;
>} else {
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Don't tell the hardware about our UAV access.
Francisco Jerez writes: > The hardware documentation relating to the UAV HW-assisted coherency > mechanism and UAV access enable bits is scarce and sometimes > contradictory, and there's quite some guesswork behind this commit, so > let me summarize the background first: HSW and later hardware have > infrastructure to support a stricter form of data coherency between > shader invocations from separate primitives. The mechanism is > controlled by the "Accesses UAV" bits on 3DSTATE_VS, _HS, _DS, _GS and > _PS (or _PS_EXTRA on BDW+), and the "UAV Coherency Required" bit on > the 3DPRIMITIVE command. > > Regardless of whether "UAV Coherency Required" is set, the hardware > fixed-function units will increment a per-stage semaphore for each > request received if "Accesses UAV" is set for the same or any lower > stage. An implicit DC flush is emitted by the lowermost stage with > "Accesses UAV" set once it's done processing the request, this also > happens regardless of the value of "UAV Coherency Required". The > completion of the DC flush will cause the same stage and all previous > ones to decrement the semaphore, marking the UAV accesses for the > primitive as coherent with L3. > > The "UAV Coherency Required" 3DPRIMITIVE bit will cause a pipeline > stall before any threads are dispatched for the first FF stage with > "Accesses UAV" set until the semaphore is cleared for the same stage. > Effectively this guarantees that UAV memory accesses performed by > previous primitives from any stage will be strictly ordered (and > thanks to the implicit DC flush visible in memory) with UAV accesses > from the following primitives. > > None of this is required by the usual image, atomic counter and SSBO > GL APIs which have very relaxed cross-primitive coherency and ordering > requirements, so we don't actually ever set the "UAV Coherency > Required" bit -- Ordering with respect to shader invocations from > previous stages on the same primitive where there is a data dependency > is of course already guaranteed as the spec requires, regardless of > this mechanism being enabled. We do set the "Accesses UAV" bits > though since my commit ac7664e493655e290783c23a0412b9c70936da50 (which > this patch partially reverts), mainly because of comments like the > following from the BDW PRM: > >> 3DSTATE_GS >>[...] >> 12 Accesses UAV >>Format: Enable >>This field must be set when GS has a UAV access. > > There are similar comments in the documentation for the other > 3DSTATE_*S commands. The "must" part is misleading and unjustified > AFAIK. Most of the "Accesses UAV" bits don't seem to have any side > effects other than the implicit DC flushes and the related > book-keeping in anticipation for a subsequent primitive with "UAV > Coherency Required" set, so in most cases they are unnecessary and may > incur a performance penalty. There is an exception though. On Gen8+ > the PS_EXTRA UAV access bit influences the calculation of the PS > UAV-only and ThreadDispatchEnable signals which on previous > generations were set explicitly by the driver, so we cannot always > avoid enabling it on the PS stage. > > The primary motivation for this change is that in fact the hardware > coherency mechanism is buggy and will cause a rather non-deterministic > hang on Gen8 when VS is the only stage with "Accesses UAV" set and the > processing of a request terminates immediately after the implicit DC > flush is sent for a previous primitive with no additional vertices > being emitted for the second primitive, what will cause the hardware > to skip sending a second DC flush and cause the VS to stall > indefinitely waiting for a response from the DC (BDWGFX HSD 1912017). > This hardware bug can be reproduced on current master with the > spec@arb_shader_image_load_store@host-mem-barrier@Indirect/RaW piglit > subtest (if you have the patience to run it a few dozen times). > > The proposed workaround is to insert CS STALLs speculatively between > 3DPRIMITIVE commands when "Accesses UAV" is enabled for the VS stage > only. Because this would affect one of the hottest paths in the > driver and likely decrease performance even further due to the > unnecessary serialization, and because we don't actually need the > implicit DC flushes, it seems better to just disable them. After mesa 11 has been branched without including this fix it should be marked: Cc: 11.0 > --- > src/mesa/drivers/dri/i965/gen7_gs_state.c | 4 +--- > src/mesa/drivers/dri/i965/gen7_vs_state.c | 4 +--- > src/mesa/drivers/dri/i965/gen7_wm_state.c | 12 > src/mesa/drivers/dri/i965/gen8_gs_state.c | 4 +--- > src/mesa/drivers/dri/i965/gen8_ps_state.c | 32 > --- > src/mesa/drivers/dri/i965/gen8_vs_state.c | 4 +--- > 6 files changed, 41 insertions(+), 19 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/gen7_gs_state.c > b/src/mesa/drivers/dri/i965/gen7_gs_state.c > index 497ecec..8d6d3fe 100644 > --- a/src/mesa/driver
Re: [Mesa-dev] [Mesa-stable] [PATCH] vc4: Initialize pack field of qreg to 0 in qir_get_temp
On 26 August 2015 at 12:52, Boyan Ding wrote: > This avoids generation of undefined packing in qir and qpu instructions, > fixing a lot of rendering errors. > > Fixes 8b36d107fdd (vc4: Pack the unorm-packing bits into a src MUL > instruction when possible.) > > Cc: mesa-sta...@lists.freedesktop.org > Signed-off-by: Boyan Ding > --- > src/gallium/drivers/vc4/vc4_qir.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/gallium/drivers/vc4/vc4_qir.c > b/src/gallium/drivers/vc4/vc4_qir.c > index 9d93071..12dedce 100644 > --- a/src/gallium/drivers/vc4/vc4_qir.c > +++ b/src/gallium/drivers/vc4/vc4_qir.c > @@ -314,6 +314,7 @@ qir_get_temp(struct vc4_compile *c) > > reg.file = QFILE_TEMP; > reg.index = c->num_temps++; > +reg.pack = 0; > Reviewed-by: Emil Velikov Eric, would you mind if we commit this ? It's been around a week and it's dead trivial. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Move gl_vert_attrib from mtypes.h to shader_enums.h
On 08/31/2015 03:57 PM, Jason Ekstrand wrote: It is a shader enum after all... --- src/glsl/shader_enums.h | 108 src/mesa/main/mtypes.h | 107 --- 2 files changed, 108 insertions(+), 107 deletions(-) OK by me. Acked-by: Brian Paul ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 16/18] i965: Prevent coordinate overflow in intel_emit_linear_blit
On Tue, Sep 01, 2015 at 02:48:20PM +0100, Emil Velikov wrote:
> On 1 August 2015 at 17:57, Emil Velikov wrote:
> > Hello all,
> >
> > On 20 July 2015 at 18:08, Anuj Phogat wrote:
> >> On Sat, Jul 18, 2015 at 1:24 AM, Chris Wilson
> >> wrote:
> >>> On Fri, Jul 17, 2015 at 05:12:54PM -0700, Anuj Phogat wrote:
> On Mon, Jul 6, 2015 at 3:33 AM, Chris Wilson
> wrote:
> > + do {
> > + /* The pitch given to the GPU must be DWORD aligned, and
> > + * we want width to match pitch. Max width is (1 << 15 - 1),
> > + * rounding that down to the nearest DWORD is 1 << 15 - 4
> > + */
> > + pitch = ROUND_DOWN_TO(MIN2(size, (1 << 15) - 64), 4);
> I understand why you are subtracting 64 in above statement, it'll
> be nice to update above comment explaining the reason.
> >>>
> >>> We use the pitch to set the copy width, so the maximum x coordinate
> >>> becomes src_x + pitch. Since src_x has a maximum value of 63, we want to
> >>> make sure that pitch is less than 32627-63. Simplified above.
> >>>
> > + height = (size < pitch || pitch == 0) ? 1 : size / pitch;
> >>> ...
> > + pitch *= height;
> > + if (size <= pitch)
> I think size < pitch will never be true. How about:
> assert(size < pitch);
> >>>
> >>> For a single row copy, size can be less than pitch.
> >> right.
> >>
> >> Reviewed-by: Anuj Phogat
> > It doesn't seem that the patch has landed in master despite the review
> > from Anuj. Is it missing something or did it fell through the cracks ?
> >
> Another humble ping !
I keep missing mesa-bugzilla updates, I see there is a tested-by now
which is what I was waiting for.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: handle SwapBytes in compressed texture get code.
On 08/31/2015 10:50 PM, Dave Airlie wrote:
This case just wasn't handled, so add support for it.
Signed-off-by: Dave Airlie
---
src/mesa/main/texgetimage.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
index 0c23687..52ed1ef 100644
--- a/src/mesa/main/texgetimage.c
+++ b/src/mesa/main/texgetimage.c
@@ -361,6 +361,13 @@ get_tex_rgba_compressed(struct gl_context *ctx, GLuint
dimensions,
tempSlice, RGBA32_FLOAT, srcStride,
width, height,
needsRebase ? rebaseSwizzle : NULL);
+
+ /* Handle byte swapping if required */
+ if (ctx->Pack.SwapBytes) {
+ _mesa_swap_bytes_2d_image(format, type, &ctx->Pack,
+ width, height, dest, NULL);
Per my comment on the first patch, could we just pass dest for both the
src and dst parameters? If so, it would probably be good to put a
comment before the function call saying it's intentional, to avoid
reader confusion.
Looks OK otherwise.
Reviewed-by: Brian Paul
+ }
+
tempSlice += 4 * width * height;
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: fix SwapBytes handling in numerous places
Yeah, this looks better. Just minor nit-picks below.
Otherwise, Reviewed-by: Brian Paul
On 08/31/2015 10:50 PM, Dave Airlie wrote:
From: Dave Airlie
In a number of places the SwapBytes handling
didn't handle cases with GL_(UN)PACK_ALIGNMENT
set and 7 byte width cases aligned to 8 bytes.
This adds a common routine to swap bytes a 2D
image and uses this code in the:
Could line-wrap closer to 75 chars.
texture store
texture getting
readpixels
and swrast drawpixels.
Signed-off-by: Dave Airlie
---
src/mesa/main/image.c | 43 ---
src/mesa/main/image.h | 20 +++-
src/mesa/main/readpix.c | 11 ++-
src/mesa/main/texgetimage.c | 14 +++---
src/mesa/main/texstore.c| 28 +---
src/mesa/swrast/s_drawpix.c | 14 +++---
6 files changed, 76 insertions(+), 54 deletions(-)
diff --git a/src/mesa/main/image.c b/src/mesa/main/image.c
index 711a190..770d89c 100644
--- a/src/mesa/main/image.c
+++ b/src/mesa/main/image.c
@@ -49,7 +49,7 @@
* \param src the array with the source data we want to byte-swap.
* \param n number of words.
*/
-void
+static void
_mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n )
Could remove _mesa_ prefix from static fn.
{
GLuint i;
@@ -58,7 +58,11 @@ _mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n )
}
}
-
+void
+_mesa_swap2(GLushort *p, GLuint n)
+{
+ _mesa_swap2_copy(p, p, n);
+}
/*
* Flip the order of the 4 bytes in each word in the given array (src) and
@@ -69,7 +73,7 @@ _mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n )
* \param src the array with the source data we want to byte-swap.
* \param n number of words.
*/
-void
+static void
_mesa_swap4_copy( GLuint *dst, GLuint *src, GLuint n )
{
GLuint i, a, b;
@@ -83,6 +87,11 @@ _mesa_swap4_copy( GLuint *dst, GLuint *src, GLuint n )
}
}
+void
+_mesa_swap4(GLuint *p, GLuint n)
+{
+ _mesa_swap4_copy(p, p, n);
+}
/**
* Return the byte offset of a specific pixel in an image (1D, 2D or 3D).
@@ -958,3 +967,31 @@ _mesa_clip_blit(struct gl_context *ctx,
return GL_TRUE;
}
+
+
+void
+_mesa_swap_bytes_2d_image(GLenum format, GLenum type,
How about putting a comment on the function. I presume the 'packing'
info applies to both the src and dest images?
+ const struct gl_pixelstore_attrib *packing,
+ GLsizei width, GLsizei height,
+ GLvoid *dst, const GLvoid *src)
+{
+ GLint swapSize = _mesa_sizeof_packed_type(type);
Can we assert(packing->SwapBytes)?
+ if (swapSize == 2 || swapSize == 4) {
+ int swapsPerPixel = _mesa_bytes_per_pixel(format, type) / swapSize;
+ int stride = _mesa_image_row_stride(packing, width, format, type);
+ int row;
+ const uint8_t *dstrow;
Remove const qualifier.
+ const uint8_t *srcrow;
+ assert(_mesa_bytes_per_pixel(format, type) % swapSize == 0);
Could also assert(swapsPerPixel >= 1)??
+ dstrow = dst;
+ srcrow = src ? src : dst;
That seems a little funny. Could the caller just pass the same address
for both dst and src? And maybe mention in the function comments that
it's legal for dst==src to do in-place byte swapping.
+ for (row = 0; row < height; row++) {
+ if (swapSize == 2)
+_mesa_swap2_copy((GLushort *) dstrow, (GLushort *)srcrow, width *
swapsPerPixel);
+ else if (swapSize == 4)
+_mesa_swap4_copy((GLuint *) dstrow, (GLuint *)srcrow, width *
swapsPerPixel);
+ dstrow += stride;
+ srcrow += stride;
+ }
+ }
+}
diff --git a/src/mesa/main/image.h b/src/mesa/main/image.h
index 501586b..b5075be 100644
--- a/src/mesa/main/image.h
+++ b/src/mesa/main/image.h
@@ -35,22 +35,11 @@ struct gl_pixelstore_attrib;
struct gl_framebuffer;
extern void
-_mesa_swap2_copy(GLushort *dst, GLushort *src, GLuint n);
+_mesa_swap2(GLushort *p, GLuint n);
extern void
-_mesa_swap4_copy(GLuint *dst, GLuint *src, GLuint n);
+_mesa_swap4(GLuint *p, GLuint n);
-static inline void
-_mesa_swap2(GLushort *p, GLuint n)
-{
- _mesa_swap2_copy(p, p, n);
-}
-
-static inline void
-_mesa_swap4(GLuint *p, GLuint n)
-{
- _mesa_swap4_copy(p, p, n);
-}
extern GLintptr
_mesa_image_offset( GLuint dimensions,
@@ -146,5 +135,10 @@ _mesa_clip_blit(struct gl_context *ctx,
GLint *srcX0, GLint *srcY0, GLint *srcX1, GLint *srcY1,
GLint *dstX0, GLint *dstY0, GLint *dstX1, GLint *dstY1);
+void
+_mesa_swap_bytes_2d_image(GLenum format, GLenum type,
+ const struct gl_pixelstore_attrib *packing,
+ GLsizei width, GLsizei height,
+ GLvoid *dst, const GLvoid *src);
#endif
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index 1277944..0ef07b5 100644
--- a/src/mesa/main/readpix.c
+++ b/src/m
Re: [Mesa-dev] [PATCH 2/3] texcompress_s3tc: fix stride checks
On Tue, 2015-09-01 at 16:41 +1000, Dave Airlie wrote:
> From: Dave Airlie
>
> The fastpath currently checks the stride != width, but
Maybe replace stride with RowLength in the line above to make things
more clear.
> if you have a RowLength of 7, and Alignment of 4, then
> that shuoldn't match.
Typo in shouldn't
> align the rowlength to the pack alignment before comparing.
Reviewed-by: Iago Toral Quiroga
BTW, it seems that at least _mesa_texstore_rgb_fxt1 in
texcompress_fxt1.c has the same issue, right?
> This fixes compressed cases in CTS packed_pixels_pixelstore
> test when SKIP_PIXELS is enabled, which causes row length
> to get set.
>
> Signed-off-by: Dave Airlie
> ---
> src/mesa/main/texcompress_s3tc.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/main/texcompress_s3tc.c
> b/src/mesa/main/texcompress_s3tc.c
> index 7ce3cb8..6cfe06a 100644
> --- a/src/mesa/main/texcompress_s3tc.c
> +++ b/src/mesa/main/texcompress_s3tc.c
> @@ -130,7 +130,7 @@ _mesa_texstore_rgb_dxt1(TEXSTORE_PARAMS)
> if (srcFormat != GL_RGB ||
> srcType != GL_UNSIGNED_BYTE ||
> ctx->_ImageTransferState ||
> - srcPacking->RowLength != srcWidth ||
> + ALIGN(srcPacking->RowLength, srcPacking->Alignment) != srcWidth ||
> srcPacking->SwapBytes) {
>/* convert image to RGB/GLubyte */
>GLubyte *tempImageSlices[1];
> @@ -187,7 +187,7 @@ _mesa_texstore_rgba_dxt1(TEXSTORE_PARAMS)
> if (srcFormat != GL_RGBA ||
> srcType != GL_UNSIGNED_BYTE ||
> ctx->_ImageTransferState ||
> - srcPacking->RowLength != srcWidth ||
> + ALIGN(srcPacking->RowLength, srcPacking->Alignment) != srcWidth ||
> srcPacking->SwapBytes) {
>/* convert image to RGBA/GLubyte */
>GLubyte *tempImageSlices[1];
> @@ -244,7 +244,7 @@ _mesa_texstore_rgba_dxt3(TEXSTORE_PARAMS)
> if (srcFormat != GL_RGBA ||
> srcType != GL_UNSIGNED_BYTE ||
> ctx->_ImageTransferState ||
> - srcPacking->RowLength != srcWidth ||
> + ALIGN(srcPacking->RowLength, srcPacking->Alignment) != srcWidth ||
> srcPacking->SwapBytes) {
>/* convert image to RGBA/GLubyte */
>GLubyte *tempImageSlices[1];
> @@ -300,7 +300,7 @@ _mesa_texstore_rgba_dxt5(TEXSTORE_PARAMS)
> if (srcFormat != GL_RGBA ||
> srcType != GL_UNSIGNED_BYTE ||
> ctx->_ImageTransferState ||
> - srcPacking->RowLength != srcWidth ||
> + ALIGN(srcPacking->RowLength, srcPacking->Alignment) != srcWidth ||
> srcPacking->SwapBytes) {
>/* convert image to RGBA/GLubyte */
>GLubyte *tempImageSlices[1];
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] meta: Compute correct buffer size with SkipRows/SkipPixels
On Tue, Sep 1, 2015 at 5:30 AM, Chris Wilson wrote: > On Tue, Sep 01, 2015 at 01:09:33PM +0100, Neil Roberts wrote: >> Good catch and it seems like a nice way to fix it. >> >> Reviewed-by: Neil Roberts >> >> I wonder if it might be worth avoiding copying the padding and pack the >> rows more tightly in the temporary buffer. Ie, we would allocate a >> buffer of align(width*cpp)*height*depth and copy the rows in one at a >> time instead of memcpying the whole buffer. As it stands for example if >> you are using the stride to make a 16x16 texture of a subregion of a >> 1024x1024 buffer we will pointlessly make a temporary buffer of 1024x16. >> I'm not saying we should hold up this fix for that though. Yeah, this is a good idea. I think I thought about that when writing the original code but it just never happened. > I've been half wondering about that as well. There seems to be a > deficit of good texture up/downloading benchmarks, that both stress > all pathways and reflect real world usage. For the latter, I wonder if > apitrace could be coaxed into service? (You need realistic GPU usage in > addition to texture transfers to answer questions such as blit vs stall.) > As for the former, we could just repurpose some of the API coverage > tests in piglit to serve as comprehensive micro-benchmarks. Yeah, the textuer upload/download testing and benchmarking situation is pretty dire. We've intruduced a number of bugs those paths without triggering a single piglit test. More tests are always welcome. > I have a v2 of this patch because piglit tells me I can't just drop > full_height on the floor - as the teximage we create is meant to be > width x full_height x 1. Oops. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/vec4: fill src_reg type using the constructor type parameter
The src_reg constructor that received the glsl_type was using it only to build the swizzle, but not to fill this->type as dst_reg is doing. This caused some type mismatch between movs and alu operations on the NIR path, so copy propagation optimization was not applied to remove unneeded movs if negate modifier was involved. This was first detected on minus (negate+add) operations. Shader DB results (taking into account only vec4): total instructions in shared programs: 20019 -> 19934 (-0.42%) instructions in affected programs: 2918 -> 2833 (-2.91%) helped:79 HURT: 0 GAINED:0 LOST: 0 --- src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp index b97b6c1..501461c 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp @@ -61,6 +61,8 @@ src_reg::src_reg(register_file file, int reg, const glsl_type *type) this->swizzle = brw_swizzle_for_size(type->vector_elements); else this->swizzle = BRW_SWIZZLE_XYZW; + if (type) + this->type = brw_type_for_base_type(type); } /** Generic unset register constructor. */ -- 2.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 37397] Expose more ES extensions that are identical to existing desktop GL functionality
https://bugs.freedesktop.org/show_bug.cgi?id=37397 Link Mauve changed: What|Removed |Added Depends on||91840 -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91840] Expose GL_KHR_debug to ES contexts
https://bugs.freedesktop.org/show_bug.cgi?id=91840 Bug ID: 91840 Summary: Expose GL_KHR_debug to ES contexts Product: Mesa Version: git Hardware: All URL: https://www.opengl.org/registry/specs/KHR/debug.txt OS: All Status: NEW Severity: enhancement Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: b...@linkmauve.fr QA Contact: mesa-dev@lists.freedesktop.org Blocks: 37397 GL_KHR_debug is currently only implemented on GL contexts, but the specification is also defined in ES terms with the addition that every symbol must end with the KHR suffix. -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] meta: Compute correct buffer size with SkipRows/SkipPixels
If the user is specifying a subregion of a buffer using SKIP_ROWS and SKIP_PIXELS, we must compute the buffer size carefully as the end of the last row may be much shorter than stride*image_height*depth. The current code tries to memcpy from beyond the end of the user data, for example causing: ==28136== Invalid read of size 8 ==28136==at 0x4C2D94E: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:915) ==28136==by 0xB4ADFE3: brw_bo_write (brw_batch.c:1856) ==28136==by 0xB5B3531: brw_buffer_data (intel_buffer_objects.c:208) ==28136==by 0xB0F6275: _mesa_buffer_data (bufferobj.c:1600) ==28136==by 0xB0F6346: _mesa_BufferData (bufferobj.c:1631) ==28136==by 0xB37A1EE: create_texture_for_pbo (meta_tex_subimage.c:103) ==28136==by 0xB37A467: _mesa_meta_pbo_TexSubImage (meta_tex_subimage.c:176) ==28136==by 0xB5C8D61: intelTexSubImage (intel_tex_subimage.c:195) ==28136==by 0xB254AB4: _mesa_texture_sub_image (teximage.c:3654) ==28136==by 0xB254C9F: texsubimage (teximage.c:3712) ==28136==by 0xB2550E9: _mesa_TexSubImage2D (teximage.c:3853) ==28136==by 0x401CA0: UploadTexSubImage2D (teximage.c:171) ==28136== Address 0xd8bfbe0 is 0 bytes after a block of size 1,024 alloc'd ==28136==at 0x4C28C20: malloc (vg_replace_malloc.c:296) ==28136==by 0x402014: PerfDraw (teximage.c:270) ==28136==by 0x402648: Draw (glmain.c:182) ==28136==by 0x8385E63: ??? (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136==by 0x83896C8: fgEnumWindows (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136==by 0x838641C: glutMainLoopEvent (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136==by 0x8386C1C: glutMainLoop (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136==by 0x4019C1: main (glmain.c:262) ==28136== ==28136== Invalid read of size 8 ==28136==at 0x4C2D940: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:915) ==28136==by 0xB4ADFE3: brw_bo_write (brw_batch.c:1856) ==28136==by 0xB5B3531: brw_buffer_data (intel_buffer_objects.c:208) ==28136==by 0xB0F6275: _mesa_buffer_data (bufferobj.c:1600) ==28136==by 0xB0F6346: _mesa_BufferData (bufferobj.c:1631) ==28136==by 0xB37A1EE: create_texture_for_pbo (meta_tex_subimage.c:103) ==28136==by 0xB37A467: _mesa_meta_pbo_TexSubImage (meta_tex_subimage.c:176) ==28136==by 0xB5C8D61: intelTexSubImage (intel_tex_subimage.c:195) ==28136==by 0xB254AB4: _mesa_texture_sub_image (teximage.c:3654) ==28136==by 0xB254C9F: texsubimage (teximage.c:3712) ==28136==by 0xB2550E9: _mesa_TexSubImage2D (teximage.c:3853) ==28136==by 0x401CA0: UploadTexSubImage2D (teximage.c:171) ==28136== Address 0xd8bfbe8 is 8 bytes after a block of size 1,024 alloc'd ==28136==at 0x4C28C20: malloc (vg_replace_malloc.c:296) ==28136==by 0x402014: PerfDraw (teximage.c:270) ==28136==by 0x402648: Draw (glmain.c:182) ==28136==by 0x8385E63: ??? (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136==by 0x83896C8: fgEnumWindows (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136==by 0x838641C: glutMainLoopEvent (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136==by 0x8386C1C: glutMainLoop (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) ==28136==by 0x4019C1: main (glmain.c:262) ==28136== Fixes regression from commit 7f396189f073d626c5f7a2c232dac92b65f5a23f Author: Jason Ekstrand Date: Mon Jan 5 18:17:04 2015 -0800 meta: Add a BlitFramebuffers-based implementation of TexSubImage v2: However, the teximage we create does need to be width x full_height x 1 Signed-off-by: Chris Wilson Cc: Jason Ekstrand Cc: Neil Roberts --- src/mesa/drivers/common/meta_tex_subimage.c | 45 +++-- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c index 16d8f5d..33c22aa 100644 --- a/src/mesa/drivers/common/meta_tex_subimage.c +++ b/src/mesa/drivers/common/meta_tex_subimage.c @@ -46,8 +46,9 @@ #include "varray.h" static struct gl_texture_image * -create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, - GLenum pbo_target, int width, int height, +create_texture_for_pbo(struct gl_context *ctx, + bool create_pbo, GLenum pbo_target, + int dims, int width, int height, int depth, GLenum format, GLenum type, const void *pixels, const struct gl_pixelstore_attrib *packing, GLuint *tmp_pbo, GLuint *tmp_tex) @@ -73,13 +74,18 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, return NULL; /* Account for SKIP_PIXELS, SKIP_ROWS, ALIGNMENT, and SKIP_IMAGES */ - pixels = _mesa_image_address3d(packing, pixels, - width, height, format, type, 0, 0, 0); + uint32_t first_pixel = _mesa_image_offset(dims, packing, width, height, + form
Re: [Mesa-dev] [PATCH] i965: Don't check for draw-time errors that cannot occur in core profile
On 08/31/2015 11:25 PM, Ilia Mirkin wrote: > On Tue, Sep 1, 2015 at 1:48 AM, Eirik Byrkjeflot Anonsen > wrote: >> Ian Romanick writes: >> >>> ping. :) >>> >>> On 08/10/2015 11:48 AM, Matt Turner wrote: On Mon, Aug 10, 2015 at 10:12 AM, Ian Romanick wrote: > From: Ian Romanick > > On many CPU-limited applications, this is *the* hot path. The idea is > to generate per-API versions of brw_draw_prims that elide some checks. > This patch removes render-mode and "is everything in VBOs" checks from > core-profile contexts. > > On my IVB laptop (which may have experienced thermal throttling): > > Gl32Batch7: 3.70955% +/- 1.11344% I'm getting 3.18414% +/- 0.587956% (n=113) on my IVB, , which probably matches your numbers depending on your value of n. > OglBatch7: 1.04398% +/- 0.772788% I'm getting 1.15377% +/- 1.05898% (n=34) on my IVB, which probably matches your numbers depending on your value of n. >>> >>> This is another thing that make me feel a little uncomfortable with the >>> way we've done performance measurements in the past. If I run my test >>> before and after this patch for 121 iterations, which I have done, I can >>> cut the data at any point and oscillate between "no difference" or X% >>> +/- some-large-fraction-of-X%. Since the before and after code for the >>> compatibility profile path should be identical, "no difference" is the >>> only believable result. >> >> That's pretty much expected, I believe. In essence, you are running 121 >> tests, each with a 95% confidence interval and so should expect >> somewhere around 5 "significant difference" results. That's not entirely >> true of course, since these are not 121 *independent* tests, but the >> basic problem remains. > > (more stats rants follow) > > While my job title has never been 'statistician', I've been around a > bunch of them. Just want to correct this... let's forget about these > tests, but instead think about coin flips (of a potentially unfair > coin). What you're doing is flipping the coin 100 times, and then > looking at the number of times it came up heads and tails. From that > you're inferring the mean of the distribution. Obviously the more > times you do the flip, the more sure you can be of your result. The > "suredness", is expressed as a confidence interval. A 95% CI means > that for 95% such experiments (i.e. "flip a coin 100 times to > determine its true heads:tails ratio"), the *true* mean of the > distribution will lie within the confidence interval (and conversely, > for 5% of such experiments, the true mean will be outside of the > interval). Note how this is _not_ "the mean has a 95% chance of lying > in the interval" or anything like that. One of these runs of 121 > iterations is a single "experiment". > > Bringing this back to what you guys are doing, which is measuring some > metric (say, time), which is hardly binomial, but one might hope that For the particular test I'm looking at here, I think it should be reasonably close. The test itself runs a small set of frames a few times (3 or 4) and logs the average FPS for the whole run. It seems like the "distribution of means is Gaussian" should apply, yeah? > the amount of time that a particular run takes on a particular machine > at a particular commit is normal. Given that, after 100 runs, you can > estimate that the "true" mean runtime is within a CI. You're then > comparing 2 CI's to determine the % change between the two > distributions, and trying to ascertain whether they are different and > by how much. > > Now, no (finite) amount of experimentation will bring you a CI of 0. > So setting out to *measure* the impact of a change is meaningless > unless you have some precise form of measurement (e.g. lines of code). > All you can do is ask the question "is the change > X". And for any > such X, you can compute the number of runs that you'd need in order to > get a CI bound that is "that tight". You could work this out > mathematically, and it depends on some of the absolute values in > question, but empirically it seems like for 50 runs, you get a CI > width of ~1%. If you're trying to demonstrate changes that are less > than 1%, or demonstrate that the change is no more than 1%, then this > is fine. If you want to demonstrate that the change is no more than > some smaller change, well, these things go like N^2, i.e. if it's 50 > runs for 1%, it's 200 runs for 0.5%, etc. That sounds familiar... that the amount of expected difference determines the lower bound on the number of required experiments. I did take "Statistics for Engineers" not that long ago. Lol. I think I still have my textbook. I'll dig around in it. For a bunch of the small changes, I don't care too much what the difference is. I just want to know whether after is better than before. > This is all still subject to the normal distribution assumption as I > mentioned earlier. Y
Re: [Mesa-dev] [PATCH] i965: Don't check for draw-time errors that cannot occur in core profile
On Tue, Sep 1, 2015 at 12:15 PM, Ian Romanick wrote: > For a bunch of the small changes, I don't care too much what the > difference is. I just want to know whether after is better than before. And that gets back to my comment that you can't *measure* the impact of a change. Not with something where the outcome is a random variable. It can't be done. All you can do is answer the question "is X's mean more than N higher than Y's mean". And you change the number of trials in an experiment depending on N. (There's also more advanced concepts like 'power' and whatnot, I've done just fine without fully understanding them, I suspect you can too.) As an aside, increasing the number of trials until you get a significant result is a great way to arrive at incorrect decisions, due to the multi-look problem (95% CI means 1/20 gives you bad results). The proper way is to decide beforehand "I care about changes >0.1%, which means I need to run 5000 trial runs" (based on the assumption that 50 runs gets you 1%). After doing the 5k runs, your CI width should be ~0.1% and you should then be able to see if the delta in means is higher or lower than that. If it's higher, then you've detected a significant change. If it's not, that btw doesn't mean "no change", just not statistically significant. There's also a procedure for the null hypothesis (i.e. is a change's impact <1%) which is basically the same thing but involves doing a few more runs (like 50% more? I forget the details). Anyways, I'm sure I've bored everyone to death with these pedantic explanations, but IME statistics is one of the most misunderstood areas of math, especially among us engineers. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH demos] egl: Remove demos using EGL_MESA_screen_surface.
Hi Matt,
2015-08-29 1:09 GMT+02:00 Matt Turner :
> The remnants of the extension were removed from Mesa in commit 7a58262e.
>
> Bugzilla: https://bugs.gentoo.org/show_bug.cgi?id=555186
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91020
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91643
Bugzilla: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=796373
Unfortunately this patch isn't enough to fix the build on Debian:
eglut.c: In function '_eglutDestroyWindow':
eglut.c:80:32: error: 'EGL_SCREEN_BIT_MESA' undeclared (first use in
this function)
_eglut->surface_type != EGL_SCREEN_BIT_MESA)
^
eglut.c:80:32: note: each undeclared identifier is reported only once
for each function it appears in
eglut.c: In function '_eglutCreateWindow':
eglut.c:178:9: error: 'EGL_SCREEN_BIT_MESA' undeclared (first use in
this function)
case EGL_SCREEN_BIT_MESA:
^
eglut.c: In function 'eglutDestroyWindow':
eglut.c:293:33: error: 'EGL_SCREEN_BIT_MESA' undeclared (first use in
this function)
if ( _eglut->surface_type != EGL_SCREEN_BIT_MESA)
^
git grep EGL_SCREEN_BIT_MESA
src/egl/eglut/eglut.c: _eglut->surface_type != EGL_SCREEN_BIT_MESA)
src/egl/eglut/eglut.c: case EGL_SCREEN_BIT_MESA:
src/egl/eglut/eglut.c: if ( _eglut->surface_type != EGL_SCREEN_BIT_MESA)
Thanks,
Andreas.
> ---
> demo1 and eglscreen didn't look useful, but maybe demo2 and demo3 are?
> They'd have to be ported to something newer -- EGL_MESA_drm_image?
>
> If someone is interested in porting any of these, I'd be happy to submit
> a smaller patch that simply adds some ifdefs to eglut_screen.c (to make
> it a no-op).
>
> src/egl/eglut/CMakeLists.txt | 6 -
> src/egl/eglut/Makefile.am| 10 +-
> src/egl/eglut/eglut_screen.c | 180 ---
> src/egl/opengl/.gitignore| 6 -
> src/egl/opengl/CMakeLists.txt| 17 -
> src/egl/opengl/Makefile.am | 11 -
> src/egl/opengl/demo1.c | 147 -
> src/egl/opengl/demo2.c | 216 -
> src/egl/opengl/demo3.c | 647
> ---
> src/egl/opengl/eglinfo.c | 45 ---
> src/egl/opengl/eglscreen.c | 120
> src/egl/opengles1/.gitignore | 4 -
> src/egl/opengles1/CMakeLists.txt | 4 -
> src/egl/opengles1/Makefile.am| 14 -
> src/egl/opengles2/.gitignore | 1 -
> src/egl/opengles2/CMakeLists.txt | 4 -
> src/egl/opengles2/Makefile.am| 7 +-
> src/egl/openvg/.gitignore| 2 -
> src/egl/openvg/CMakeLists.txt| 4 -
> src/egl/openvg/Makefile.am | 8 -
> 20 files changed, 4 insertions(+), 1449 deletions(-)
> delete mode 100644 src/egl/eglut/eglut_screen.c
> delete mode 100644 src/egl/opengl/demo1.c
> delete mode 100644 src/egl/opengl/demo2.c
> delete mode 100644 src/egl/opengl/demo3.c
> delete mode 100644 src/egl/opengl/eglscreen.c
>
> diff --git a/src/egl/eglut/CMakeLists.txt b/src/egl/eglut/CMakeLists.txt
> index a885977..0417b48 100644
> --- a/src/egl/eglut/CMakeLists.txt
> +++ b/src/egl/eglut/CMakeLists.txt
> @@ -9,9 +9,3 @@ if (X11_FOUND)
> install (TARGETS eglut_x11 DESTINATION ${LIBDIR})
> endif (BUILD_SHARED_LIBS)
> endif (X11_FOUND)
> -
> -add_library (eglut_screen eglut.h eglut.c eglutint.h eglut_screen.c)
> -target_link_libraries (eglut_screen ${EGL_LIBRARIES})
> -if (BUILD_SHARED_LIBS)
> - install (TARGETS eglut_screen DESTINATION ${LIBDIR})
> -endif (BUILD_SHARED_LIBS)
> diff --git a/src/egl/eglut/Makefile.am b/src/egl/eglut/Makefile.am
> index 2d2f2af..b765069 100644
> --- a/src/egl/eglut/Makefile.am
> +++ b/src/egl/eglut/Makefile.am
> @@ -33,17 +33,12 @@ if HAVE_WAYLAND
> eglut_wayland = libeglut_wayland.la
> endif
>
> -noinst_LTLIBRARIES = libeglut_screen.la $(eglut_x11) $(eglut_wayland)
> +noinst_LTLIBRARIES = $(eglut_x11) $(eglut_wayland)
> endif
>
> -libeglut_screen_la_SOURCES = \
> - eglut.c \
> - eglut.h \
> - eglutint.h \
> - eglut_screen.c
> -
> libeglut_x11_la_SOURCES = \
> eglut.c \
> + eglut.h \
> eglutint.h \
> eglut_x11.c
> libeglut_x11_la_CFLAGS = $(X11_CFLAGS) $(EGL_CFLAGS)
> @@ -52,6 +47,7 @@ libeglut_x11_la_LIBADD = $(X11_LIBS) $(EGL_LIBS)
>
> libeglut_wayland_la_SOURCES = \
> eglut.c \
> + eglut.h \
> eglutint.h \
> eglut_wayland.c
>
> diff --git a/src/egl/eglut/eglut_screen.c b/src/egl/eglut/eglut_screen.c
> deleted file mode 100644
> index 021a8f1..000
> --- a/src/egl/eglut/eglut_screen.c
> +++ /dev/null
> @@ -1,180 +0,0 @@
> -/*
> - * Copyright (C) 2010 LunarG Inc.
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distr
Re: [Mesa-dev] [PATCH v2] meta: Compute correct buffer size with SkipRows/SkipPixels
Looks good to me. Chris Wilson writes: > @@ -324,10 +340,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, > GLuint dims, > * property. > */ > image_height = packing->ImageHeight == 0 ? height : packing->ImageHeight; > - full_height = image_height * (depth - 1) + height; > > pbo_tex_image = create_texture_for_pbo(ctx, false, GL_PIXEL_PACK_BUFFER, > - width, full_height * depth, > + dims, width, height, depth, It looks like this bit was wrong previously and this patch fixes it. I think it would previously end up creating a texture that is height*depth² tall! However it probably wouldn't really matter because as far as I can tell this path is only used for an existing PBO so it wouldn't end up allocating any buffers or copying any data, it would just have the height wrong in the sampler state. Reviewed-by: Neil Roberts Regards, - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/dri: Use packed RGB formats
On Mon, Aug 10, 2015 at 5:44 AM, Michel Dänzer wrote:
> From: Michel Dänzer
>
> Fixes Gallium based DRI drivers failing to load on big endian hosts
> because they can't find any matching fbconfigs.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71789
> Signed-off-by: Michel Dänzer
> ---
> src/gallium/state_trackers/dri/dri2.c | 26 +-
> src/gallium/state_trackers/dri/dri_drawable.c | 8
> 2 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/src/gallium/state_trackers/dri/dri2.c
> b/src/gallium/state_trackers/dri/dri2.c
> index 91b4431..fae100e 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -188,10 +188,10 @@ dri2_drawable_get_buffers(struct dri_drawable *drawable,
> * may occur as the stvis->color_format.
> */
>switch(format) {
> - case PIPE_FORMAT_B8G8R8A8_UNORM:
> + case PIPE_FORMAT_BGRA_UNORM:
> depth = 32;
> break;
> - case PIPE_FORMAT_B8G8R8X8_UNORM:
> + case PIPE_FORMAT_BGRX_UNORM:
> depth = 24;
> break;
>case PIPE_FORMAT_B5G6R5_UNORM:
> @@ -261,13 +261,13 @@ dri_image_drawable_get_buffers(struct dri_drawable
> *drawable,
>case PIPE_FORMAT_B5G6R5_UNORM:
> image_format = __DRI_IMAGE_FORMAT_RGB565;
> break;
> - case PIPE_FORMAT_B8G8R8X8_UNORM:
> + case PIPE_FORMAT_BGRX_UNORM:
> image_format = __DRI_IMAGE_FORMAT_XRGB;
> break;
> - case PIPE_FORMAT_B8G8R8A8_UNORM:
> + case PIPE_FORMAT_BGRA_UNORM:
> image_format = __DRI_IMAGE_FORMAT_ARGB;
> break;
> - case PIPE_FORMAT_R8G8B8A8_UNORM:
> + case PIPE_FORMAT_RGBA_UNORM:
> image_format = __DRI_IMAGE_FORMAT_ABGR;
> break;
>default:
> @@ -314,10 +314,10 @@ dri2_allocate_buffer(__DRIscreen *sPriv,
>
> switch (format) {
>case 32:
> - pf = PIPE_FORMAT_B8G8R8A8_UNORM;
> + pf = PIPE_FORMAT_BGRA_UNORM;
> break;
>case 24:
> - pf = PIPE_FORMAT_B8G8R8X8_UNORM;
> + pf = PIPE_FORMAT_BGRX_UNORM;
> break;
>case 16:
> pf = PIPE_FORMAT_Z16_UNORM;
> @@ -724,13 +724,13 @@ dri2_create_image_from_winsys(__DRIscreen *_screen,
>pf = PIPE_FORMAT_B5G6R5_UNORM;
>break;
> case __DRI_IMAGE_FORMAT_XRGB:
> - pf = PIPE_FORMAT_B8G8R8X8_UNORM;
> + pf = PIPE_FORMAT_BGRX_UNORM;
>break;
> case __DRI_IMAGE_FORMAT_ARGB:
> - pf = PIPE_FORMAT_B8G8R8A8_UNORM;
> + pf = PIPE_FORMAT_BGRA_UNORM;
>break;
> case __DRI_IMAGE_FORMAT_ABGR:
> - pf = PIPE_FORMAT_R8G8B8A8_UNORM;
> + pf = PIPE_FORMAT_RGBA_UNORM;
>break;
> default:
>pf = PIPE_FORMAT_NONE;
> @@ -845,13 +845,13 @@ dri2_create_image(__DRIscreen *_screen,
>pf = PIPE_FORMAT_B5G6R5_UNORM;
>break;
> case __DRI_IMAGE_FORMAT_XRGB:
> - pf = PIPE_FORMAT_B8G8R8X8_UNORM;
> + pf = PIPE_FORMAT_BGRX_UNORM;
>break;
> case __DRI_IMAGE_FORMAT_ARGB:
> - pf = PIPE_FORMAT_B8G8R8A8_UNORM;
> + pf = PIPE_FORMAT_BGRA_UNORM;
>break;
> case __DRI_IMAGE_FORMAT_ABGR:
> - pf = PIPE_FORMAT_R8G8B8A8_UNORM;
> + pf = PIPE_FORMAT_RGBA_UNORM;
>break;
> default:
>pf = PIPE_FORMAT_NONE;
> diff --git a/src/gallium/state_trackers/dri/dri_drawable.c
> b/src/gallium/state_trackers/dri/dri_drawable.c
> index 0d2929a..f0cc4a2 100644
> --- a/src/gallium/state_trackers/dri/dri_drawable.c
> +++ b/src/gallium/state_trackers/dri/dri_drawable.c
> @@ -231,11 +231,11 @@ dri_set_tex_buffer2(__DRIcontext *pDRICtx, GLint target,
>if (format == __DRI_TEXTURE_FORMAT_RGB) {
> /* only need to cover the formats recognized by dri_fill_st_visual
> */
> switch (internal_format) {
> - case PIPE_FORMAT_B8G8R8A8_UNORM:
> -internal_format = PIPE_FORMAT_B8G8R8X8_UNORM;
> + case PIPE_FORMAT_BGRA_UNORM:
> +internal_format = PIPE_FORMAT_BGRX_UNORM;
> break;
> - case PIPE_FORMAT_A8R8G8B8_UNORM:
> -internal_format = PIPE_FORMAT_X8R8G8B8_UNORM;
> + case PIPE_FORMAT_ARGB_UNORM:
> +internal_format = PIPE_FORMAT_XRGB_UNORM;
> break;
> default:
> break;
This dri_drawable.c hunk is unnecessary. I wrote the same patch for
dri2.c though. Any reason it didn't get pushed?
-ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] meta: Compute correct buffer size with SkipRows/SkipPixels
On Tue, Sep 01, 2015 at 05:44:30PM +0100, Neil Roberts wrote: > Looks good to me. > > Chris Wilson writes: > > > @@ -324,10 +340,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, > > GLuint dims, > > * property. > > */ > > image_height = packing->ImageHeight == 0 ? height : > > packing->ImageHeight; > > - full_height = image_height * (depth - 1) + height; > > > > pbo_tex_image = create_texture_for_pbo(ctx, false, GL_PIXEL_PACK_BUFFER, > > - width, full_height * depth, > > + dims, width, height, depth, > > It looks like this bit was wrong previously and this patch fixes it. I > think it would previously end up creating a texture that is > height*depth² tall! However it probably wouldn't really matter because > as far as I can tell this path is only used for an existing PBO so it > wouldn't end up allocating any buffers or copying any data, it would > just have the height wrong in the sampler state. Hmm, it is a good point though as we don't check that the full_height does comply with the sampler limit. That could cause some problems in a few corner cases. Thanks for the review, -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH demos] egl: Remove demos using EGL_MESA_screen_surface.
On Tue, Sep 1, 2015 at 9:42 AM, Andreas Boll wrote: > Hi Matt, > > 2015-08-29 1:09 GMT+02:00 Matt Turner : >> The remnants of the extension were removed from Mesa in commit 7a58262e. >> >> Bugzilla: https://bugs.gentoo.org/show_bug.cgi?id=555186 >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91020 >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91643 > > Bugzilla: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=796373 > > Unfortunately this patch isn't enough to fix the build on Debian: > > eglut.c: In function '_eglutDestroyWindow': > eglut.c:80:32: error: 'EGL_SCREEN_BIT_MESA' undeclared (first use in > this function) > _eglut->surface_type != EGL_SCREEN_BIT_MESA) > ^ > eglut.c:80:32: note: each undeclared identifier is reported only once > for each function it appears in > eglut.c: In function '_eglutCreateWindow': > eglut.c:178:9: error: 'EGL_SCREEN_BIT_MESA' undeclared (first use in > this function) > case EGL_SCREEN_BIT_MESA: > ^ > eglut.c: In function 'eglutDestroyWindow': > eglut.c:293:33: error: 'EGL_SCREEN_BIT_MESA' undeclared (first use in > this function) > if ( _eglut->surface_type != EGL_SCREEN_BIT_MESA) > ^ > > git grep EGL_SCREEN_BIT_MESA > src/egl/eglut/eglut.c: _eglut->surface_type != EGL_SCREEN_BIT_MESA) > src/egl/eglut/eglut.c: case EGL_SCREEN_BIT_MESA: > src/egl/eglut/eglut.c: if ( _eglut->surface_type != EGL_SCREEN_BIT_MESA) > > > Thanks, > Andreas. Thanks Andreas. I came to the same conclusion. I'll squash a patch to remove the uses from eglut.c as well. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glapi: Inline x86_64_current_tls().
I looked at the change that added this (97185bf2), and there was no
explanation why it was done this way... and the rest of the
MAPI_MODE_BRIDGE stuff seems fairly nuts. This change, however, seems
reasonable.
Reviewed-by: Ian Romanick
On 08/28/2015 11:47 AM, Matt Turner wrote:
> ---
> Here's another glapi patch that I never got upstream.
>
> src/mapi/entry_x86-64_tls.h | 10 ++
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/src/mapi/entry_x86-64_tls.h b/src/mapi/entry_x86-64_tls.h
> index 5c03b04..38faccc 100644
> --- a/src/mapi/entry_x86-64_tls.h
> +++ b/src/mapi/entry_x86-64_tls.h
> @@ -46,13 +46,6 @@ __asm__(".text\n"
>
> #ifndef MAPI_MODE_BRIDGE
>
> -__asm__("x86_64_current_tls:\n\t"
> - "movq " ENTRY_CURRENT_TABLE "@GOTTPOFF(%rip), %rax\n\t"
> - "ret");
> -
> -extern unsigned long
> -x86_64_current_tls();
> -
> #include
> #include "u_execmem.h"
>
> @@ -90,7 +83,8 @@ entry_generate(int slot)
> char *code;
> mapi_func entry;
>
> - addr = x86_64_current_tls();
> + __asm__("movq " ENTRY_CURRENT_TABLE "@GOTTPOFF(%%rip), %0"
> + : "=r" (addr));
> if ((addr >> 32) != 0x)
>return NULL;
> addr &= 0x;
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH demos] egl: Remove demos using EGL_MESA_screen_surface.
2015-09-01 18:55 GMT+02:00 Matt Turner : > On Tue, Sep 1, 2015 at 9:42 AM, Andreas Boll > wrote: >> Hi Matt, >> >> 2015-08-29 1:09 GMT+02:00 Matt Turner : >>> The remnants of the extension were removed from Mesa in commit 7a58262e. >>> >>> Bugzilla: https://bugs.gentoo.org/show_bug.cgi?id=555186 >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91020 >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91643 >> >> Bugzilla: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=796373 >> >> Unfortunately this patch isn't enough to fix the build on Debian: >> >> eglut.c: In function '_eglutDestroyWindow': >> eglut.c:80:32: error: 'EGL_SCREEN_BIT_MESA' undeclared (first use in >> this function) >> _eglut->surface_type != EGL_SCREEN_BIT_MESA) >> ^ >> eglut.c:80:32: note: each undeclared identifier is reported only once >> for each function it appears in >> eglut.c: In function '_eglutCreateWindow': >> eglut.c:178:9: error: 'EGL_SCREEN_BIT_MESA' undeclared (first use in >> this function) >> case EGL_SCREEN_BIT_MESA: >> ^ >> eglut.c: In function 'eglutDestroyWindow': >> eglut.c:293:33: error: 'EGL_SCREEN_BIT_MESA' undeclared (first use in >> this function) >> if ( _eglut->surface_type != EGL_SCREEN_BIT_MESA) >> ^ >> >> git grep EGL_SCREEN_BIT_MESA >> src/egl/eglut/eglut.c: _eglut->surface_type != EGL_SCREEN_BIT_MESA) >> src/egl/eglut/eglut.c: case EGL_SCREEN_BIT_MESA: >> src/egl/eglut/eglut.c: if ( _eglut->surface_type != EGL_SCREEN_BIT_MESA) >> >> >> Thanks, >> Andreas. > > Thanks Andreas. I came to the same conclusion. I'll squash a patch to > remove the uses from eglut.c as well. With this patch [1] squashed in: Reviewed-by: Andreas Boll Tested-by: Andreas Boll [1] https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=ae997dc2325c0b74ad1c9b6ee8542d44354c60b4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] mesa/formats: 8-bit channel integer formats addition
For 5&6, I didn't check every tiny detail, but looks good to me. Reviewed-by: Brian Paul On 08/25/2015 07:14 PM, Dave Airlie wrote: From: Dave Airlie Add enough 8-bit channel formats to handle all the different things CTS throws at us. Signed-off-by: Dave Airlie --- src/mesa/main/formats.c | 43 +++ src/mesa/main/formats.csv| 4 src/mesa/main/formats.h | 5 + src/mesa/main/glformats.c| 8 src/mesa/swrast/s_texfetch.c | 4 5 files changed, 64 insertions(+) diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c index e73a549..e151f30 100644 --- a/src/mesa/main/formats.c +++ b/src/mesa/main/formats.c @@ -883,6 +883,10 @@ _mesa_uncompressed_format_to_type_and_comps(mesa_format format, case MESA_FORMAT_R8G8B8X8_UNORM: case MESA_FORMAT_B8G8R8X8_UNORM: case MESA_FORMAT_X8R8G8B8_UNORM: + case MESA_FORMAT_A8B8G8R8_UINT: + case MESA_FORMAT_R8G8B8A8_UINT: + case MESA_FORMAT_B8G8R8A8_UINT: + case MESA_FORMAT_A8R8G8B8_UINT: *datatype = GL_UNSIGNED_BYTE; *comps = 4; return; @@ -1992,6 +1996,45 @@ _mesa_format_matches_format_and_type(mesa_format mesa_format, case MESA_FORMAT_R5G5B5A1_UINT: return format == GL_RGBA_INTEGER && type == GL_UNSIGNED_SHORT_1_5_5_5_REV; + case MESA_FORMAT_A8B8G8R8_UINT: + if (format == GL_RGBA_INTEGER && type == GL_UNSIGNED_INT_8_8_8_8 && !swapBytes) + return GL_TRUE; + + if (format == GL_RGBA_INTEGER && type == GL_UNSIGNED_INT_8_8_8_8_REV && swapBytes) + return GL_TRUE; + return GL_FALSE; + + case MESA_FORMAT_A8R8G8B8_UINT: + if (format == GL_BGRA_INTEGER && type == GL_UNSIGNED_INT_8_8_8_8 && + !swapBytes) + return GL_TRUE; + + if (format == GL_BGRA_INTEGER && type == GL_UNSIGNED_INT_8_8_8_8_REV && + swapBytes) + return GL_TRUE; + + return GL_FALSE; + + case MESA_FORMAT_R8G8B8A8_UINT: + if (format == GL_RGBA_INTEGER && type == GL_UNSIGNED_INT_8_8_8_8_REV && + !swapBytes) + return GL_TRUE; + + if (format == GL_RGBA_INTEGER && type == GL_UNSIGNED_INT_8_8_8_8 && swapBytes) + return GL_TRUE; + + return GL_FALSE; + + case MESA_FORMAT_B8G8R8A8_UINT: + if (format == GL_BGRA_INTEGER && type == GL_UNSIGNED_INT_8_8_8_8_REV && + !swapBytes) + return GL_TRUE; + + if (format == GL_BGRA_INTEGER && type == GL_UNSIGNED_INT_8_8_8_8 && swapBytes) + return GL_TRUE; + + return GL_FALSE; + case MESA_FORMAT_R9G9B9E5_FLOAT: return format == GL_RGB && type == GL_UNSIGNED_INT_5_9_9_9_REV && !swapBytes; diff --git a/src/mesa/main/formats.csv b/src/mesa/main/formats.csv index d30b0a9..3e70655 100644 --- a/src/mesa/main/formats.csv +++ b/src/mesa/main/formats.csv @@ -186,6 +186,10 @@ MESA_FORMAT_RGBX_FLOAT32 , array , 1, 1, f32 , f32 , f32 , x32 MESA_FORMAT_Z_FLOAT32 , array , 1, 1, f32 , , , , x___, zs # Packed signed/unsigned non-normalized integer formats +MESA_FORMAT_A8B8G8R8_UINT , packed, 1, 1, u8 , u8 , u8 , u8 , wzyx, rgb +MESA_FORMAT_A8R8G8B8_UINT , packed, 1, 1, u8 , u8 , u8 , u8 , yzwx, rgb +MESA_FORMAT_R8G8B8A8_UINT , packed, 1, 1, u8 , u8 , u8 , u8 , xyzw, rgb +MESA_FORMAT_B8G8R8A8_UINT , packed, 1, 1, u8 , u8 , u8 , u8 , zyxw, rgb MESA_FORMAT_B10G10R10A2_UINT , packed, 1, 1, u10 , u10 , u10 , u2 , zyxw, rgb MESA_FORMAT_R10G10B10A2_UINT , packed, 1, 1, u10 , u10 , u10 , u2 , xyzw, rgb MESA_FORMAT_A2B10G10R10_UINT , packed, 1, 1, u2 , u10 , u10 , u10 , wzyx, rgb diff --git a/src/mesa/main/formats.h b/src/mesa/main/formats.h index dff5980..17cb091 100644 --- a/src/mesa/main/formats.h +++ b/src/mesa/main/formats.h @@ -470,6 +470,11 @@ typedef enum MESA_FORMAT_Z_FLOAT32, /* Packed signed/unsigned non-normalized integer formats */ + + MESA_FORMAT_A8B8G8R8_UINT,/* */ + MESA_FORMAT_A8R8G8B8_UINT,/* */ + MESA_FORMAT_R8G8B8A8_UINT,/* */ + MESA_FORMAT_B8G8R8A8_UINT,/* */ MESA_FORMAT_B10G10R10A2_UINT, /* AARR GGBB */ MESA_FORMAT_R10G10B10A2_UINT, /* AABB GGRR */ MESA_FORMAT_A2B10G10R10_UINT, /* RRGG BBAA */ diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c index 672532f..cef831c 100644 --- a/src/mesa/main/glformats.c +++ b/src/mesa/main/glformats.c @@ -2818,6 +2818,10 @@ _mesa_format_from_format_and_type(GLenum format, GLenum type) return MESA_FORMAT_A8R8G8B8_UNORM; else if (format == GL_ABGR_EXT) return MESA_FORMAT_R8G8B8A8_UN
[Mesa-dev] [PATCH] mesa/pbo: Handle zero height or depth when validating PBO access
It's legal to call glTexSubImage with zero values for the width, height or depth. Previously this was breaking the PBO access validation because it tries to work out the last pixel accessed by getting the pixel at height-1 and depth-1 which would end up with bogus values. This was causing GL errors to be generated during the Piglit texsubimage test, although the test was passing anyway. --- src/mesa/main/pbo.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c index 0c16025..f71cbc0 100644 --- a/src/mesa/main/pbo.c +++ b/src/mesa/main/pbo.c @@ -108,8 +108,11 @@ _mesa_validate_pbo_access(GLuint dimensions, format, type, 0, 0, 0); /* get the offset to just past the last pixel we'll read/write */ - end = _mesa_image_offset(dimensions, pack, width, height, - format, type, depth-1, height-1, width); + if (depth == 0 || height == 0) + end = start; + else + end = _mesa_image_offset(dimensions, pack, width, height, +format, type, depth-1, height-1, width); start += offset; end += offset; -- 1.9.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/12] bmake inspired fixes
On Tue, Sep 1, 2015 at 6:41 AM, Emil Velikov wrote: > If you'd like some objective justification why these patches make > things better, please give me some merits that I can check against. As I've stated... quite a few times now, I don't like that the series removes the AM_V_LEX/AM_V_YACC when they've caused no problems, I don't like that the series removes a GNU make construct simply because it is a GNU make construct [half of the others]. I also haven't heard any reasons why making the build system compatible with BSD make is a useful thing to do -- and justification is a required part of getting a patch accepted. To be honest I'm so tired of dealing with this that I'm just going to ignore the lack of justification and review it. [01/12] mapi: automake: inline glapi_gen_mapi define I don't like it. Push it anyway after you fix the indentation of the second line to match 04/12. [02/12] xmlpool: remove LOCALEDIR variable/fix bmake I don't feel I understand the implications. [03/12] util: automake: rework the format_srgb.c rule Reviewed-by: Matt Turner [04/12] mapi: automake: rework the *api/glapi_mapi_tmp.h rules Reviewed-by: Matt Turner [05/12] mapi: automake: rework the source generation rules Reviewed-by: Matt Turner [06/12] mesa: automake: rework the source generation rules Reviewed-by: Matt Turner [07/12] glsl: automake: remove custom AM_V_LEX/YACC Not interested [08/12] glsl: automake: rework the sources generation rules Requires rebasing if 06/12 is dropped. Seems fine then. [09/12] glsl: automake: reuse $(NIR_GENERATED_FILES) where possible Reviewed-by: Matt Turner [10/12] glsl: build: use makefile.sources variables when possible Reviewed-by: Matt Turner [11/12] glsl: build: remove bogus dependency Reviewed-by: Matt Turner [12/12] auxiliary: fix the generated sources rules No idea what this one is doing. > Alternatively I'll push these within a few days. Given the number of times I've previously expressed to you that I didn't like the series, I think that would be improper. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/pbo: Handle zero height or depth when validating PBO access
On Tue, Sep 1, 2015 at 1:40 PM, Neil Roberts wrote: > It's legal to call glTexSubImage with zero values for the width, > height or depth. Previously this was breaking the PBO access > validation because it tries to work out the last pixel accessed by > getting the pixel at height-1 and depth-1 which would end up with > bogus values. > > This was causing GL errors to be generated during the Piglit > texsubimage test, although the test was passing anyway. > --- > src/mesa/main/pbo.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c > index 0c16025..f71cbc0 100644 > --- a/src/mesa/main/pbo.c > +++ b/src/mesa/main/pbo.c > @@ -108,8 +108,11 @@ _mesa_validate_pbo_access(GLuint dimensions, >format, type, 0, 0, 0); > > /* get the offset to just past the last pixel we'll read/write */ > - end = _mesa_image_offset(dimensions, pack, width, height, > - format, type, depth-1, height-1, width); > + if (depth == 0 || height == 0) Why not width == 0 as well? You could probably just do return GL_TRUE; in that case as well, and then call _mesa_image_offset unconditionally for both end/start. IMHO simpler and more efficient. > + end = start; > + else > + end = _mesa_image_offset(dimensions, pack, width, height, I've always found a single space is enough for me... apparently not for the original author of this code (which you then just indented). > +format, type, depth-1, height-1, width); > > start += offset; > end += offset; > -- > 1.9.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/vec4: fill src_reg type using the constructor type parameter
On Tue, Sep 1, 2015 at 8:02 AM, Alejandro Piñeiro wrote: > The src_reg constructor that received the glsl_type was using it > only to build the swizzle, but not to fill this->type as dst_reg > is doing. > > This caused some type mismatch between movs and alu operations > on the NIR path, so copy propagation optimization was not applied > to remove unneeded movs if negate modifier was involved. This was > first detected on minus (negate+add) operations. > > Shader DB results (taking into account only vec4): > > total instructions in shared programs: 20019 -> 19934 (-0.42%) > instructions in affected programs: 2918 -> 2833 (-2.91%) > helped:79 > HURT: 0 > GAINED:0 > LOST: 0 > --- How silly. :) Thanks for finding that. Reviewed-by: Matt Turner ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/8] nouveau: add support for vaapi
On Tue, Sep 1, 2015 at 4:58 AM, Julien Isorce wrote: > Hi, > > Thx for the comments. I forgot to mention that there is nothing really new > in these patches. > All the low level code relative to nouveau was already there. > The patches are almost "just" moving code in order to call nouveau_vp3_bsp > and nvc0_decoder_bsp > multiple times between each begin/end frame. But any refactoring is a source > of regressions so the "just" may be not that trivial :) > > I made sure that vdpau was still working. Actually it was more than that, > without vdpau I think I would not have been able to do this refactoring. I > also compared that for one begin/end frame the nouveau_bo buffer contains > the same data for vdpau and vaapi. I dumped them for the same stream and > several seconds and there were all the same. Great. Please also ensure that you test all 4 supported formats -- mpeg2, mpeg4p2 (aka divx/xvid/etc), mpeg4p10 (aka h264), and vc1. You don't have to do thorough testing for all of them, but just making sure that they look like they might work would be nice. > > Would it be ok if I squash the patches like this (4 patches instead of 8): > > A: Common to nvc0 and nv98: > nouveau: extract memcpy loop from nouveau_vp3_bsp > nouveau: remove nouveau_vp3_bsp to use begin/next/end > > B: Only for nvc0 but same split can be applied to nv98: > +nvc0(-nouveau): split nvc0_decoder_bsp in begin/next/end > +nvc0(-nouveau): remove nvc0_decoder_bsp and use begin/next/end instead > +nvc0(-nouveau): preserve content buffer when calling nvc0_decoder_bsp_next > nvc0: implement pipe_video_codec::begin_frame/end_frame > > C: Common to nvc0 and nv98: > nouveau: fix chunk decoding by updating number of slices > > D: Common to nvc0 and nv98: > build: enable st/va with nouveau driver > > I can still squash A and B, no pb. Let me know. I think it's fine as you have it above. Basically I want to avoid the previous situation of patches like 1. add some code that's not used 2. change use of old code to new code 3. remove old code Since that is just very difficult to review. All 3 of those need to be in the same patch since then it's easy to ensure that identical functionality is being preserved. > Also should I use "--in-reply-to" or should I submit a new patch set since > some will be squashed ? We're not too strict on those things. I think a new patchset, with 'git format-patch -v2' or something to distinguish it, would be just fine. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/pbo: Handle zero height or depth when validating PBO access
On Tue, Sep 01, 2015 at 06:40:39PM +0100, Neil Roberts wrote: > It's legal to call glTexSubImage with zero values for the width, > height or depth. Previously this was breaking the PBO access > validation because it tries to work out the last pixel accessed by > getting the pixel at height-1 and depth-1 which would end up with > bogus values. Hmm. this was the style I just copied to find the access size for the meta_tex_image patch. Do we need to filter out no-op glTexSubImages in the driver backend or will the core? At the moment, it looks like this request will be passed along and processed by the drivers. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] Fix for clang compiler issue as reported in Bug 91826
On Tue, Sep 1, 2015 at 10:10 AM, Albert Freeman
wrote:
> Clang tryed to declare the non type member of struct module (enum type type)
> (in clover/core/module.hpp) instead of a variable of type enum (enum type).
>
> Signed-off-by: Albert Freeman
> ---
Thanks for your first patch :)
The subject and commit message don't really match the Mesa styles. We
prefix the subject with the area that the code affects -- in this case
"clover: "
How about
clover: Avoid using typename with ... TBD
clover/core/module.hpp declares "type" to be "enum type". Using the
typename keyword, clang attempted to declare the variable with the
same type as "module" instead of "module::argument::type". Just use
enum to avoid this.
Reviewed by Serge Martin
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91826
But I'm curious if this code is exposing a bug in clang, relies on a
language extension clang doesn't implement, or is simply wrong? Curro
(Cc'd) will probably know.
> src/gallium/state_trackers/clover/llvm/invocation.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index 7c23a27..d74b50d 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -465,7 +465,7 @@ namespace {
> const bool is_write_only = access_qual == "write_only";
> const bool is_read_only = access_qual == "read_only";
>
> -typename module::argument::type marg_type;
> +enum module::argument::type marg_type;
> if (is_image2d && is_read_only) {
> marg_type = module::argument::image2d_rd;
> } else if (is_image2d && is_write_only) {
> --
> 2.5.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 12/12] auxiliary: fix the generated sources rules
On 07/17/2015 10:29 AM, Emil Velikov wrote: > Signed-off-by: Emil Velikov > --- > src/gallium/auxiliary/Makefile.am | 29 + > 1 file changed, 17 insertions(+), 12 deletions(-) > > diff --git a/src/gallium/auxiliary/Makefile.am > b/src/gallium/auxiliary/Makefile.am > index 04f77d0..a728162 100644 > --- a/src/gallium/auxiliary/Makefile.am > +++ b/src/gallium/auxiliary/Makefile.am > @@ -38,18 +38,23 @@ libgallium_la_SOURCES += \ > > endif > > -indices/u_indices_gen.c: $(srcdir)/indices/u_indices_gen.py > - $(AM_V_at)$(MKDIR_P) indices > - $(AM_V_GEN) $(PYTHON2) $< > $@ > - > -indices/u_unfilled_gen.c: $(srcdir)/indices/u_unfilled_gen.py > - $(AM_V_at)$(MKDIR_P) indices > - $(AM_V_GEN) $(PYTHON2) $< > $@ > - > -util/u_format_table.c: $(srcdir)/util/u_format_table.py > $(srcdir)/util/u_format_pack.py $(srcdir)/util/u_format_parse.py > $(srcdir)/util/u_format.csv > - $(AM_V_at)$(MKDIR_P) util > - $(AM_V_GEN) $(PYTHON2) $(srcdir)/util/u_format_table.py > $(srcdir)/util/u_format.csv > $@ > - > +MKDIR_GEN = $(AM_V_at)$(MKDIR_P) $(@D) > +PYTHON_GEN = $(AM_V_GEN)$(PYTHON2) $(PYTHON_FLAGS) So, this changes the dependencies (see below), and adds $(@D) to mkdir and $(PYTHON_FLAGS) to python... maybe a brief sentence in the commit message explaining why. > + > +indices/u_indices_gen.c: indices/u_indices_gen.py Shouldn't all of these still depend on $(srcdir)/... ? > + $(MKDIR_GEN) > + $(PYTHON_GEN) $(srcdir)/indices/u_indices_gen.py > $@ > + > +indices/u_unfilled_gen.c: indices/u_unfilled_gen.py > + $(MKDIR_GEN) > + $(PYTHON_GEN) $(srcdir)/indices/u_unfilled_gen.py > $@ > + > +util/u_format_table.c: util/u_format_table.py \ > + util/u_format_pack.py \ > + util/u_format_parse.py \ > + util/u_format.csv > + $(MKDIR_GEN) > + $(PYTHON_GEN) $(srcdir)/util/u_format_table.py > $(srcdir)/util/u_format.csv > $@ > > noinst_LTLIBRARIES += libgalliumvl_stub.la > libgalliumvl_stub_la_SOURCES = \ > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91840] Expose GL_KHR_debug to ES contexts
https://bugs.freedesktop.org/show_bug.cgi?id=91840 Jose Fonseca changed: What|Removed |Added CC||jfons...@vmware.com -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] Fix for clang compiler issue as reported in Bug 91826
Matt Turner writes:
> On Tue, Sep 1, 2015 at 10:10 AM, Albert Freeman
> wrote:
>> Clang tryed to declare the non type member of struct module (enum type type)
>> (in clover/core/module.hpp) instead of a variable of type enum (enum type).
>>
>> Signed-off-by: Albert Freeman
>> ---
>
> Thanks for your first patch :)
>
> The subject and commit message don't really match the Mesa styles. We
> prefix the subject with the area that the code affects -- in this case
> "clover: "
>
> How about
>
> clover: Avoid using typename with ... TBD
>
> clover/core/module.hpp declares "type" to be "enum type". Using the
> typename keyword, clang attempted to declare the variable with the
> same type as "module" instead of "module::argument::type". Just use
> enum to avoid this.
>
> Reviewed by Serge Martin
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91826
>
>
>
> But I'm curious if this code is exposing a bug in clang, relies on a
> language extension clang doesn't implement, or is simply wrong? Curro
> (Cc'd) will probably know.
>
Heh, I suspect the previous code was ill-formed and Clang correctly
rejected it. I may be wrong but I don't think that typename is supposed
to alter the look-up rules to give you a type which was previously
hidden by a variable declaration of the same name (as e.g. enum or
struct do).
With Matt's suggestions this patch is:
Reviewed-by: Francisco Jerez
>> src/gallium/state_trackers/clover/llvm/invocation.cpp | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> index 7c23a27..d74b50d 100644
>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> @@ -465,7 +465,7 @@ namespace {
>> const bool is_write_only = access_qual == "write_only";
>> const bool is_read_only = access_qual == "read_only";
>>
>> -typename module::argument::type marg_type;
>> +enum module::argument::type marg_type;
>> if (is_image2d && is_read_only) {
>> marg_type = module::argument::image2d_rd;
>> } else if (is_image2d && is_write_only) {
>> --
>> 2.5.0
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] winsys/radeon: remove exported buffers from the cache
From: Marek Olšák
Cc: 11.0
---
src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
index 600ced9..2878c8f 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
@@ -1150,6 +1150,9 @@ static boolean radeon_winsys_bo_get_handle(struct
pb_buffer *buffer,
memset(&flink, 0, sizeof(flink));
+if ((void*)bo != (void*)buffer)
+ pb_cache_manager_remove_buffer(buffer);
+
if (whandle->type == DRM_API_HANDLE_TYPE_SHARED) {
if (!bo->flink_name) {
flink.handle = bo->handle;
--
2.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] gallium/pb_bufmgr_cache: add a way to remove buffers from the cache explicitly
From: Marek Olšák
This must be done before exporting a buffer as dmabuf fds, because
we lose track of who is using it and can't trust the reference counter.
Cc: 11.0
---
src/gallium/auxiliary/pipebuffer/pb_bufmgr.h | 5 +++
src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c | 42 ++
2 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/src/gallium/auxiliary/pipebuffer/pb_bufmgr.h
b/src/gallium/auxiliary/pipebuffer/pb_bufmgr.h
index 147ce39..1638d96 100644
--- a/src/gallium/auxiliary/pipebuffer/pb_bufmgr.h
+++ b/src/gallium/auxiliary/pipebuffer/pb_bufmgr.h
@@ -166,6 +166,11 @@ pb_cache_manager_create(struct pb_manager *provider,
unsigned bypass_usage,
uint64_t maximum_cache_size);
+/**
+ * Remove a buffer from the cache, but keep it alive.
+ */
+void
+pb_cache_manager_remove_buffer(struct pb_buffer *buf);
struct pb_fence_ops;
diff --git a/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c
b/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c
index 3b35049..cc8ae84 100644
--- a/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c
+++ b/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c
@@ -104,18 +104,42 @@ pb_cache_manager(struct pb_manager *mgr)
}
+static void
+_pb_cache_manager_remove_buffer_locked(struct pb_cache_buffer *buf)
+{
+ struct pb_cache_manager *mgr = buf->mgr;
+
+ if (buf->head.next) {
+ LIST_DEL(&buf->head);
+ assert(mgr->numDelayed);
+ --mgr->numDelayed;
+ mgr->cache_size -= buf->base.size;
+ }
+ buf->mgr = NULL;
+}
+
+void
+pb_cache_manager_remove_buffer(struct pb_buffer *pb_buf)
+{
+ struct pb_cache_buffer *buf = (struct pb_cache_buffer*)pb_buf;
+ struct pb_cache_manager *mgr = buf->mgr;
+
+ if (!mgr)
+ return;
+
+ pipe_mutex_lock(mgr->mutex);
+ _pb_cache_manager_remove_buffer_locked(buf);
+ pipe_mutex_unlock(mgr->mutex);
+}
+
/**
* Actually destroy the buffer.
*/
static inline void
_pb_cache_buffer_destroy(struct pb_cache_buffer *buf)
{
- struct pb_cache_manager *mgr = buf->mgr;
-
- LIST_DEL(&buf->head);
- assert(mgr->numDelayed);
- --mgr->numDelayed;
- mgr->cache_size -= buf->base.size;
+ if (buf->mgr)
+ _pb_cache_manager_remove_buffer_locked(buf);
assert(!pipe_is_referenced(&buf->base.reference));
pb_reference(&buf->buffer, NULL);
FREE(buf);
@@ -156,6 +180,12 @@ pb_cache_buffer_destroy(struct pb_buffer *_buf)
struct pb_cache_buffer *buf = pb_cache_buffer(_buf);
struct pb_cache_manager *mgr = buf->mgr;
+ if (!mgr) {
+ pb_reference(&buf->buffer, NULL);
+ FREE(buf);
+ return;
+ }
+
pipe_mutex_lock(mgr->mutex);
assert(!pipe_is_referenced(&buf->base.reference));
--
2.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] winsys/amdgpu: remove exported buffers from the cache
From: Marek Olšák
Cc: 11.0
---
src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
index 50c42e3..fe55dc3 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
@@ -684,6 +684,9 @@ static boolean amdgpu_bo_get_handle(struct pb_buffer
*buffer,
enum amdgpu_bo_handle_type type;
int r;
+ if ((void*)bo != (void*)buffer)
+ pb_cache_manager_remove_buffer(buffer);
+
switch (whandle->type) {
case DRM_API_HANDLE_TYPE_SHARED:
type = amdgpu_bo_handle_type_gem_flink_name;
--
2.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Don't check for draw-time errors that cannot occur in core profile
Ilia Mirkin writes:
> On Tue, Sep 1, 2015 at 12:15 PM, Ian Romanick wrote:
>> For a bunch of the small changes, I don't care too much what the
>> difference is. I just want to know whether after is better than before.
>
> And that gets back to my comment that you can't *measure* the impact
> of a change. Not with something where the outcome is a random
> variable. It can't be done.
>
> All you can do is answer the question "is X's mean more than N higher
> than Y's mean". And you change the number of trials in an experiment
> depending on N. (There's also more advanced concepts like 'power' and
> whatnot, I've done just fine without fully understanding them, I
> suspect you can too.)
Power is (IIRC) just an opposite of the p-value. That is, the p-value
gives you the probability of false positives and the power is the
probability of false negatives (or rather 1-power, but whatever). The
complication is that you usually choose the p-value (typically 0.05)
while the power has to be calculated.
> As an aside, increasing the number of trials until you get a
> significant result is a great way to arrive at incorrect decisions,
> due to the multi-look problem (95% CI means 1/20 gives you bad
> results). The proper way is to decide beforehand "I care about changes
>>0.1%, which means I need to run 5000 trial runs"
The trick could be to run a sequence of tests until you find how many
trials are needed for significance. Then you can check if you get
repeatable results with that many trials, in which case you are safe.
The key word is of course "repeatable". If you have a correctly executed
test that gives repeatable "significant difference", it usually doesn't
matter too much how you figured out which parameters were needed.
("usually", because you could run into a choice of parameters that
invalidates the whole test. But just increasing the number of trials
shouldn't do that.)
Which brings us to the clear value of multiple people running similar
tests and getting similar results. That strengthens the conclusion
significantly.
> (based on the
> assumption that 50 runs gets you 1%). After doing the 5k runs, your CI
> width should be ~0.1% and you should then be able to see if the delta
> in means is higher or lower than that. If it's higher, then you've
> detected a significant change. If it's not, that btw doesn't mean "no
> change", just not statistically significant. There's also a procedure
> for the null hypothesis (i.e. is a change's impact <1%) which is
> basically the same thing but involves doing a few more runs (like 50%
> more? I forget the details).
Hmm, you could just formulate your null hypothesis as "the change is
greater than 1%" and then test that normally.
> Anyways, I'm sure I've bored everyone to death with these pedantic
> explanations, but IME statistics is one of the most misunderstood
> areas of math, especially among us engineers.
>
> -ilia
What, statistics boring? No way! :)
eirik
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Don't check for draw-time errors that cannot occur in core profile
On Tue, Sep 1, 2015 at 3:45 PM, Eirik Byrkjeflot Anonsen
wrote:
> Ilia Mirkin writes:
>
>> On Tue, Sep 1, 2015 at 12:15 PM, Ian Romanick wrote:
>>> For a bunch of the small changes, I don't care too much what the
>>> difference is. I just want to know whether after is better than before.
>>
>> And that gets back to my comment that you can't *measure* the impact
>> of a change. Not with something where the outcome is a random
>> variable. It can't be done.
>>
>> All you can do is answer the question "is X's mean more than N higher
>> than Y's mean". And you change the number of trials in an experiment
>> depending on N. (There's also more advanced concepts like 'power' and
>> whatnot, I've done just fine without fully understanding them, I
>> suspect you can too.)
>
> Power is (IIRC) just an opposite of the p-value. That is, the p-value
> gives you the probability of false positives and the power is the
> probability of false negatives (or rather 1-power, but whatever). The
> complication is that you usually choose the p-value (typically 0.05)
> while the power has to be calculated.
>
>> As an aside, increasing the number of trials until you get a
>> significant result is a great way to arrive at incorrect decisions,
>> due to the multi-look problem (95% CI means 1/20 gives you bad
>> results). The proper way is to decide beforehand "I care about changes
>>>0.1%, which means I need to run 5000 trial runs"
>
> The trick could be to run a sequence of tests until you find how many
> trials are needed for significance. Then you can check if you get
> repeatable results with that many trials, in which case you are safe.
Well, it's all very deterministic, just simple math. Which is annoying
to do, especially in terms of percentage change. So the empirical is
fine. But if you don't decide on a desired CI width up front, you're
in for a spankin' from your friendly local statistician.
>
> The key word is of course "repeatable". If you have a correctly executed
> test that gives repeatable "significant difference", it usually doesn't
> matter too much how you figured out which parameters were needed.
> ("usually", because you could run into a choice of parameters that
> invalidates the whole test. But just increasing the number of trials
> shouldn't do that.)
>
> Which brings us to the clear value of multiple people running similar
> tests and getting similar results. That strengthens the conclusion
> significantly.
>
>> (based on the
>> assumption that 50 runs gets you 1%). After doing the 5k runs, your CI
>> width should be ~0.1% and you should then be able to see if the delta
>> in means is higher or lower than that. If it's higher, then you've
>> detected a significant change. If it's not, that btw doesn't mean "no
>> change", just not statistically significant. There's also a procedure
>> for the null hypothesis (i.e. is a change's impact <1%) which is
>> basically the same thing but involves doing a few more runs (like 50%
>> more? I forget the details).
>
> Hmm, you could just formulate your null hypothesis as "the change is
> greater than 1%" and then test that normally.
Yeah, but there's a difference between "the change is not
statistically greater than 1%" and "the change is statistically
smaller than 1%".
>
>> Anyways, I'm sure I've bored everyone to death with these pedantic
>> explanations, but IME statistics is one of the most misunderstood
>> areas of math, especially among us engineers.
>>
>> -ilia
>
> What, statistics boring? No way! :)
>
> eirik
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r600g: Simplify out a couple of unnecessary branches
Pushed, thanks. Marek On Tue, Sep 1, 2015 at 10:38 AM, Edward O'Callaghan wrote: > From: Edward O'Callaghan > > Signed-off-by: Edward O'Callaghan > --- > src/gallium/drivers/r600/r600_shader.c | 8 ++-- > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/src/gallium/drivers/r600/r600_shader.c > b/src/gallium/drivers/r600/r600_shader.c > index b7d7828..1ab389c 100644 > --- a/src/gallium/drivers/r600/r600_shader.c > +++ b/src/gallium/drivers/r600/r600_shader.c > @@ -1966,13 +1966,9 @@ static int r600_shader_from_tgsi(struct r600_context > *rctx, > > ctx.nliterals = 0; > ctx.literals = NULL; > - shader->fs_write_all = FALSE; > - if (ctx.info.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS]) > - shader->fs_write_all = TRUE; > > - shader->vs_position_window_space = FALSE; > - if (ctx.info.properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION]) > - shader->vs_position_window_space = TRUE; > + shader->fs_write_all = > ctx.info.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS]; > + shader->vs_position_window_space = > ctx.info.properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION]; > > if (shader->vs_as_gs_a) > vs_add_primid_output(&ctx, key.vs.prim_id_out); > -- > 2.4.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r600g: Simplify out a couple of unnecessary branches
Please stop using a non-existent email address. alterapraxis.com doesn't exist. Marek On Tue, Sep 1, 2015 at 10:38 AM, Edward O'Callaghan wrote: > From: Edward O'Callaghan > > Signed-off-by: Edward O'Callaghan > --- > src/gallium/drivers/r600/r600_shader.c | 8 ++-- > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/src/gallium/drivers/r600/r600_shader.c > b/src/gallium/drivers/r600/r600_shader.c > index b7d7828..1ab389c 100644 > --- a/src/gallium/drivers/r600/r600_shader.c > +++ b/src/gallium/drivers/r600/r600_shader.c > @@ -1966,13 +1966,9 @@ static int r600_shader_from_tgsi(struct r600_context > *rctx, > > ctx.nliterals = 0; > ctx.literals = NULL; > - shader->fs_write_all = FALSE; > - if (ctx.info.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS]) > - shader->fs_write_all = TRUE; > > - shader->vs_position_window_space = FALSE; > - if (ctx.info.properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION]) > - shader->vs_position_window_space = TRUE; > + shader->fs_write_all = > ctx.info.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS]; > + shader->vs_position_window_space = > ctx.info.properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION]; > > if (shader->vs_as_gs_a) > vs_add_primid_output(&ctx, key.vs.prim_id_out); > -- > 2.4.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91747] Ubuntu 15.04/Oibaf PPA - Unity bar not transparent
https://bugs.freedesktop.org/show_bug.cgi?id=91747
--- Comment #3 from Brian Paul ---
Thanks to Sinclair Yeh's detective work, it appears that the unity shell is
disabling transparency when it finds "LLVM" in the GL_RENDERER string.
We've seen the same issue here with our latest VMware driver which includes the
LLVM version in the GL_RENDERER string. Sinclair found this code in the Unity
shell in plugins/unityshell/src/unityshell.cpp:
//In case of software rendering then enable lowgfx mode.
std::string renderer = ANSI_TO_TCHAR(NUX_REINTERPRET_CAST(const char *,
glGetString(GL_RENDERER)));
if (renderer.find("Software Rasterizer") != std::string::npos ||
renderer.find("Mesa X11") != std::string::npos ||
renderer.find("LLVM") != std::string::npos ||
renderer.find("on softpipe") != std::string::npos ||
(getenv("UNITY_LOW_GFX_MODE") != NULL &&
atoi(getenv("UNITY_LOW_GFX_MODE")) == 1) ||
optionGetLowGraphicsMode())
{
unity_settings_.SetLowGfxMode(true);
}
It looks like Marek added code in the r600 driver to include LLVM version info
in the renderer string too:
commit a3723fb9e32ab114dcffcf74946def92647c5f03
Author: Marek Olšák
Date: Mon Jul 20 00:15:59 2015 +0200
gallium/radeon: add DRM and LLVM version to the renderer string
So, if the reporter of this bug is using the r600 driver, that could explain
things. He mentions a change in June/July.
Clearly, the unity code above is pretty dodgy. I'll see if we can report the
issue upstream.
BTW, I tried pinging the bug reporter but email to g9352...@trbvm.com bounces.
--
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
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radeonsi: only use new versions of LLVM image and sample intrinsics
From: Marek Olšák
Just a cleanup I had made a long time ago and forgot about.
---
src/gallium/drivers/radeonsi/si_shader.c | 468 +--
1 file changed, 191 insertions(+), 277 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_shader.c
b/src/gallium/drivers/radeonsi/si_shader.c
index ab5b3ee..7550a42 100644
--- a/src/gallium/drivers/radeonsi/si_shader.c
+++ b/src/gallium/drivers/radeonsi/si_shader.c
@@ -2256,6 +2256,64 @@ static bool tgsi_is_shadow_sampler(unsigned target)
target == TGSI_TEXTURE_SHADOWRECT;
}
+static bool tgsi_is_array_sampler(unsigned target)
+{
+ return target == TGSI_TEXTURE_1D_ARRAY ||
+ target == TGSI_TEXTURE_SHADOW1D_ARRAY ||
+ target == TGSI_TEXTURE_2D_ARRAY ||
+ target == TGSI_TEXTURE_SHADOW2D_ARRAY ||
+ target == TGSI_TEXTURE_CUBE_ARRAY ||
+ target == TGSI_TEXTURE_SHADOWCUBE_ARRAY ||
+ target == TGSI_TEXTURE_2D_ARRAY_MSAA;
+}
+
+static void set_tex_fetch_args(struct gallivm_state *gallivm,
+ struct lp_build_emit_data *emit_data,
+ unsigned opcode, unsigned target,
+ LLVMValueRef res_ptr, LLVMValueRef samp_ptr,
+ LLVMValueRef *param, unsigned count,
+ unsigned dmask)
+{
+ unsigned num_args;
+ unsigned is_rect = target == TGSI_TEXTURE_RECT;
+ LLVMTypeRef i32 = LLVMInt32TypeInContext(gallivm->context);
+
+ /* Pad to power of two vector */
+ while (count < util_next_power_of_two(count))
+ param[count++] = LLVMGetUndef(i32);
+
+ /* Texture coordinates. */
+ if (count > 1)
+ emit_data->args[0] = lp_build_gather_values(gallivm, param,
count);
+ else
+ emit_data->args[0] = param[0];
+
+ /* Resource. */
+ emit_data->args[1] = res_ptr;
+ num_args = 2;
+
+ if (opcode == TGSI_OPCODE_TXF || opcode == TGSI_OPCODE_TXQ)
+ emit_data->dst_type = LLVMVectorType(i32, 4);
+ else {
+ emit_data->dst_type = LLVMVectorType(
+ LLVMFloatTypeInContext(gallivm->context), 4);
+
+ emit_data->args[num_args++] = samp_ptr;
+ }
+
+ emit_data->args[num_args++] = lp_build_const_int32(gallivm, dmask);
+ emit_data->args[num_args++] = lp_build_const_int32(gallivm, is_rect);
/* unorm */
+ emit_data->args[num_args++] = lp_build_const_int32(gallivm, 0); /* r128
*/
+ emit_data->args[num_args++] = lp_build_const_int32(gallivm,
+ tgsi_is_array_sampler(target)); /* da */
+ emit_data->args[num_args++] = lp_build_const_int32(gallivm, 0); /* glc
*/
+ emit_data->args[num_args++] = lp_build_const_int32(gallivm, 0); /* slc
*/
+ emit_data->args[num_args++] = lp_build_const_int32(gallivm, 0); /* tfe
*/
+ emit_data->args[num_args++] = lp_build_const_int32(gallivm, 0); /* lwe
*/
+
+ emit_data->arg_count = num_args;
+}
+
static const struct lp_build_tgsi_action tex_action;
static void tex_fetch_args(
@@ -2264,6 +2322,7 @@ static void tex_fetch_args(
{
struct si_shader_context *si_shader_ctx = si_shader_context(bld_base);
struct gallivm_state *gallivm = bld_base->base.gallivm;
+ LLVMBuilderRef builder = gallivm->builder;
const struct tgsi_full_instruction * inst = emit_data->inst;
unsigned opcode = inst->Instruction.Opcode;
unsigned target = inst->Texture.Texture;
@@ -2278,6 +2337,8 @@ static void tex_fetch_args(
unsigned num_deriv_channels = 0;
bool has_offset = HAVE_LLVM >= 0x0305 ? inst->Texture.NumOffsets > 0 :
false;
LLVMValueRef res_ptr, samp_ptr, fmask_ptr = NULL;
+ LLVMTypeRef i32 = LLVMInt32TypeInContext(gallivm->context);
+ unsigned dmask = 0xf;
sampler_src = emit_data->inst->Instruction.NumSrcRegs - 1;
sampler_index = emit_data->inst->Src[sampler_src].Register.Index;
@@ -2308,6 +2369,43 @@ static void tex_fetch_args(
fmask_ptr = si_shader_ctx->resources[SI_FMASK_TEX_OFFSET +
sampler_index];
}
+ if (opcode == TGSI_OPCODE_TXQ) {
+ if (target == TGSI_TEXTURE_BUFFER) {
+ LLVMTypeRef v8i32 = LLVMVectorType(i32, 8);
+
+ /* Read the size from the buffer descriptor directly. */
+ LLVMValueRef res = LLVMBuildBitCast(builder, res_ptr,
v8i32, "");
+ LLVMValueRef size = LLVMBuildExtractElement(builder,
res,
+
lp_build_const_int32(gallivm, 6), "");
+
+ if (si_shader_ctx->screen->b.chip_class >= VI) {
+ /* On VI, the descriptor contains the size in
bytes,
+* but TXQ must return the size in elements.
+
[Mesa-dev] [PATCH] gallium: remove support for predicates from TGSI
From: Marek Olšák
Neved used.
---
Take 2. Let's see how people feel about TGSI now.
src/gallium/auxiliary/gallivm/lp_bld_limits.h | 4 -
src/gallium/auxiliary/gallivm/lp_bld_tgsi.h| 2 -
src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c| 46 ---
src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c | 6 +-
src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c| 138 ++-
src/gallium/auxiliary/tgsi/tgsi_build.c| 66 -
src/gallium/auxiliary/tgsi/tgsi_build.h| 3 -
src/gallium/auxiliary/tgsi/tgsi_dump.c | 24
src/gallium/auxiliary/tgsi/tgsi_exec.c | 59
src/gallium/auxiliary/tgsi/tgsi_exec.h | 7 -
src/gallium/auxiliary/tgsi/tgsi_parse.c| 4 -
src/gallium/auxiliary/tgsi/tgsi_parse.h| 1 -
src/gallium/auxiliary/tgsi/tgsi_sanity.c | 1 -
src/gallium/auxiliary/tgsi/tgsi_strings.c | 1 -
src/gallium/auxiliary/tgsi/tgsi_text.c | 37 -
src/gallium/auxiliary/tgsi/tgsi_ureg.c | 84 +---
src/gallium/auxiliary/tgsi/tgsi_ureg.h | 149 +
src/gallium/docs/source/screen.rst | 1 -
src/gallium/drivers/freedreno/freedreno_screen.c | 2 -
src/gallium/drivers/i915/i915_fpc.h| 1 -
src/gallium/drivers/i915/i915_screen.c | 2 -
src/gallium/drivers/ilo/ilo_screen.c | 2 -
src/gallium/drivers/ilo/shader/toy_tgsi.c | 5 -
.../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 10 +-
src/gallium/drivers/nouveau/nv30/nv30_screen.c | 2 -
src/gallium/drivers/nouveau/nv50/nv50_screen.c | 2 -
src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 2 -
src/gallium/drivers/r300/r300_screen.c | 4 -
src/gallium/drivers/r600/r600_pipe.c | 2 -
src/gallium/drivers/r600/r600_shader.c | 4 -
src/gallium/drivers/radeonsi/si_pipe.c | 2 -
src/gallium/drivers/svga/svga_screen.c | 4 -
src/gallium/include/pipe/p_defines.h | 1 -
src/gallium/include/pipe/p_shader_tokens.h | 25 +---
src/gallium/state_trackers/nine/nine_shader.c | 18 +--
35 files changed, 27 insertions(+), 694 deletions(-)
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_limits.h
b/src/gallium/auxiliary/gallivm/lp_bld_limits.h
index 571c615..da774bf 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_limits.h
+++ b/src/gallium/auxiliary/gallivm/lp_bld_limits.h
@@ -49,8 +49,6 @@
#define LP_MAX_TGSI_IMMEDIATES 4096
-#define LP_MAX_TGSI_PREDS 16
-
#define LP_MAX_TGSI_CONSTS 4096
#define LP_MAX_TGSI_CONST_BUFFERS 16
@@ -109,8 +107,6 @@ gallivm_get_shader_param(enum pipe_shader_cap param)
return PIPE_MAX_CONSTANT_BUFFERS;
case PIPE_SHADER_CAP_MAX_TEMPS:
return LP_MAX_TGSI_TEMPS;
- case PIPE_SHADER_CAP_MAX_PREDS:
- return LP_MAX_TGSI_PREDS;
case PIPE_SHADER_CAP_TGSI_CONT_SUPPORTED:
return 1;
case PIPE_SHADER_CAP_INDIRECT_INPUT_ADDR:
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h
b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h
index 2ca9c61..a48f008 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h
+++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h
@@ -455,7 +455,6 @@ struct lp_build_tgsi_soa_context
LLVMValueRef immediates[LP_MAX_INLINED_IMMEDIATES][TGSI_NUM_CHANNELS];
LLVMValueRef temps[LP_MAX_INLINED_TEMPS][TGSI_NUM_CHANNELS];
LLVMValueRef addr[LP_MAX_TGSI_ADDRS][TGSI_NUM_CHANNELS];
- LLVMValueRef preds[LP_MAX_TGSI_PREDS][TGSI_NUM_CHANNELS];
/* We allocate/use this array of temps if (1 << TGSI_FILE_TEMPORARY) is
* set in the indirect_files field.
@@ -549,7 +548,6 @@ struct lp_build_tgsi_aos_context
LLVMValueRef immediates[LP_MAX_INLINED_IMMEDIATES];
LLVMValueRef temps[LP_MAX_INLINED_TEMPS];
LLVMValueRef addr[LP_MAX_TGSI_ADDRS];
- LLVMValueRef preds[LP_MAX_TGSI_PREDS];
/* We allocate/use this array of temps if (1 << TGSI_FILE_TEMPORARY) is
* set in the indirect_files field.
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c
b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c
index 610283d..8094efc 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c
@@ -256,10 +256,6 @@ lp_emit_store_aos(
ptr = bld->addr[reg->Indirect.Index];
break;
- case TGSI_FILE_PREDICATE:
- ptr = bld->preds[reg->Register.Index];
- break;
-
default:
assert(0);
return;
@@ -267,43 +263,6 @@ lp_emit_store_aos(
if (!ptr)
return;
- /*
-* Predicate
-*/
-
- if (inst->Instruction.Predicate) {
- LLVMValueRef pred;
-
- assert(inst->Predicate.Index < LP_MAX_TGSI_PREDS);
-
- pred = LLVMBuildLoad(builder,
- bld->preds[inst->Predicate.Index], "");
-
- /*
- * Convert the value to an integer
Re: [Mesa-dev] [PATCH] gallium: remove support for predicates from TGSI
Hi, There is predication in direct3D9 HLSL, however we don't implement them and never had any complaint about that. Reason is that HLSL Microsoft compiler doesn't generate any predication. I read somewhere they did this choice because for drivers it is possible to lower efficiently if conditions to predicates if hardware supports it, but not the opposite. If never a d3d9 apps needs predication, we'll implement them with if conditions. I'm totally fine with predication support being removed from TGSI. A little nitpicking: instead of setting lower_preds to true, just remove it. The variable isn't used. Axel Davy On 01/09/2015 23:19, Marek Olšák wrote : From: Marek Olšák Neved used. --- ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91840] Expose GL_KHR_debug to ES contexts
https://bugs.freedesktop.org/show_bug.cgi?id=91840 --- Comment #1 from Ian Romanick --- I think the main effort would be in creating piglit tests. Someone would also need to run the Khronos conformance tests. Either I or someone at Intel should be able to do that once patches are available to enable the extension. -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/pbo: Handle zero height or depth when validating PBO access
On 09/01/2015 11:17 AM, Chris Wilson wrote: > On Tue, Sep 01, 2015 at 06:40:39PM +0100, Neil Roberts wrote: >> It's legal to call glTexSubImage with zero values for the width, >> height or depth. Previously this was breaking the PBO access >> validation because it tries to work out the last pixel accessed by >> getting the pixel at height-1 and depth-1 which would end up with >> bogus values. > > Hmm. this was the style I just copied to find the access size for the > meta_tex_image patch. Do we need to filter out no-op glTexSubImages in > the driver backend or will the core? At the moment, it looks like this > request will be passed along and processed by the drivers. It seems like it should be handled in the core, and it looks like _mesa_tex_sub_image is already doing that. Note the "if (width > 0 && height > 0 && depth > 0)" check. What is the callstack that gets here with height or depth as zero? That seems fishy. > -Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 12/12] auxiliary: fix the generated sources rules
Hi Ian, On 1 September 2015 at 19:30, Ian Romanick wrote: > On 07/17/2015 10:29 AM, Emil Velikov wrote: >> Signed-off-by: Emil Velikov >> --- >> src/gallium/auxiliary/Makefile.am | 29 + >> 1 file changed, 17 insertions(+), 12 deletions(-) >> >> diff --git a/src/gallium/auxiliary/Makefile.am >> b/src/gallium/auxiliary/Makefile.am >> index 04f77d0..a728162 100644 >> --- a/src/gallium/auxiliary/Makefile.am >> +++ b/src/gallium/auxiliary/Makefile.am >> @@ -38,18 +38,23 @@ libgallium_la_SOURCES += \ >> >> endif >> >> -indices/u_indices_gen.c: $(srcdir)/indices/u_indices_gen.py >> - $(AM_V_at)$(MKDIR_P) indices >> - $(AM_V_GEN) $(PYTHON2) $< > $@ >> - >> -indices/u_unfilled_gen.c: $(srcdir)/indices/u_unfilled_gen.py >> - $(AM_V_at)$(MKDIR_P) indices >> - $(AM_V_GEN) $(PYTHON2) $< > $@ >> - >> -util/u_format_table.c: $(srcdir)/util/u_format_table.py >> $(srcdir)/util/u_format_pack.py $(srcdir)/util/u_format_parse.py >> $(srcdir)/util/u_format.csv >> - $(AM_V_at)$(MKDIR_P) util >> - $(AM_V_GEN) $(PYTHON2) $(srcdir)/util/u_format_table.py >> $(srcdir)/util/u_format.csv > $@ >> - >> +MKDIR_GEN = $(AM_V_at)$(MKDIR_P) $(@D) >> +PYTHON_GEN = $(AM_V_GEN)$(PYTHON2) $(PYTHON_FLAGS) > > So, this changes the dependencies (see below), and adds $(@D) to mkdir > and $(PYTHON_FLAGS) to python... maybe a brief sentence in the commit > message explaining why. > I felt that repeating the description might be an overkill considering most of this series does is almost identical. How about the following (also mentioned in the cover letter) "auxiliary: rework the python generated sources rules There are a few grains this commit aims to resolve: One can generalise the mkdir rule to a simple MKDIR_P $(@D) which will expand appropriately for even if we change the subdir name, and/or add new rules. We can also drop the explicit $(srcdir) prefix for the dependency rules, they they are not strictly required, nor used elsewhere in mesa. Finally replace $< with explicit filename to be consistent through the file, and honour PYTHON_FLAGS. " >> + >> +indices/u_indices_gen.c: indices/u_indices_gen.py > > Shouldn't all of these still depend on $(srcdir)/... ? > As mentioned above - I'm 98% sure that it's optional, plus other places in mesa do not seem to use it. If you insist I can add it back. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 91840] Expose GL_KHR_debug to ES contexts
https://bugs.freedesktop.org/show_bug.cgi?id=91840 --- Comment #2 from Timothy Arceri --- There are patches on the list not sure if they still apply: http://lists.freedesktop.org/archives/mesa-dev/2014-August/066941.html The piglit tests have already been updated to support ES, this was done at the same time as the above Mesa changes. The piglit updates landed the Mesa changes didn't. -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] Revert "glsl: check total count of multi-slot double vertex attribs"
This reverts commit ad208d975a6d3aebe14f7c2c16039ee200d8b30c.
This code attempted to ensure that we didn't use more than the maximum
number of attribute slots, but it did not consider attribute aliasing so
it would miscount slots used by aliased attributes and incorrectly
reject valid shaders.
Worse, the code to handle attribute double slots was just working around
a mistake in count_attribute_slots() (Fixed by previous patch).
Moreover the check seems to be unnecessary, since setting attributes via
ARB_explicit_attrib_location properly checks if the attribute is in
bounds, and the linker properly checks if other attributes fit into the
limits.
Fixes deQP functional.attribute_location.bind_aliasing.max_cond_* tests.
Cc: "10.6 11.0"
Conflicts:
src/glsl/linker.cpp
---
src/glsl/linker.cpp | 41 +
1 file changed, 1 insertion(+), 40 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 47f7d25..ed28049 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -2389,7 +2389,6 @@ assign_attribute_or_color_locations(gl_shader_program
*prog,
} to_assign[16];
unsigned num_attr = 0;
- unsigned total_attribs_size = 0;
foreach_in_list(ir_instruction, node, sh->ir) {
ir_variable *const var = node->as_variable();
@@ -2450,41 +2449,12 @@ assign_attribute_or_color_locations(gl_shader_program
*prog,
return false;
}
- const unsigned slots = var->type->count_attribute_slots();
-
- /* From GL4.5 core spec, section 11.1.1 (Vertex Attributes):
- *
- * "A program with more than the value of MAX_VERTEX_ATTRIBS active
- * attribute variables may fail to link, unless device-dependent
- * optimizations are able to make the program fit within available
- * hardware resources. For the purposes of this test, attribute variables
- * of the type dvec3, dvec4, dmat2x3, dmat2x4, dmat3, dmat3x4, dmat4x3,
- * and dmat4 may count as consuming twice as many attributes as
equivalent
- * single-precision types. While these types use the same number of
- * generic attributes as their single-precision equivalents,
- * implementations are permitted to consume two single-precision vectors
- * of internal storage for each three- or four-component double-precision
- * vector."
- * Until someone has a good reason in Mesa, enforce that now.
- */
- if (target_index == MESA_SHADER_VERTEX) {
-total_attribs_size += slots;
-if (var->type->without_array() == glsl_type::dvec3_type ||
-var->type->without_array() == glsl_type::dvec4_type ||
-var->type->without_array() == glsl_type::dmat2x3_type ||
-var->type->without_array() == glsl_type::dmat2x4_type ||
-var->type->without_array() == glsl_type::dmat3_type ||
-var->type->without_array() == glsl_type::dmat3x4_type ||
-var->type->without_array() == glsl_type::dmat4x3_type ||
-var->type->without_array() == glsl_type::dmat4_type)
- total_attribs_size += slots;
- }
-
/* If the variable is not a built-in and has a location statically
* assigned in the shader (presumably via a layout qualifier), make sure
* that it doesn't collide with other assigned locations. Otherwise,
* add it to the list of variables that need linker-assigned locations.
*/
+ const unsigned slots = var->type->count_attribute_slots();
if (var->data.location != -1) {
if (var->data.location >= generic_base && var->data.index < 1) {
/* From page 61 of the OpenGL 4.0 spec:
@@ -2604,15 +2574,6 @@ assign_attribute_or_color_locations(gl_shader_program
*prog,
num_attr++;
}
- if (target_index == MESA_SHADER_VERTEX) {
- if (total_attribs_size > max_index) {
-linker_error(prog,
- "attempt to use %d vertex attribute slots only %d
available ",
- total_attribs_size, max_index);
-return false;
- }
- }
-
/* If all of the attributes were assigned locations by the application (or
* are built-in attributes with fixed locations), return early. This should
* be the common case.
--
2.4.6
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] glsl: Properly count doubles as taking twice the number of attrib slots.
---
I checked the uses of count_attribute_slots() and it looks like they're
expecting this already, but these two patches definitely need testing on
a driver that supports fp64.
src/glsl/glsl_types.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index 755618a..cd7fef5 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -1386,9 +1386,11 @@ glsl_type::count_attribute_slots() const
case GLSL_TYPE_INT:
case GLSL_TYPE_FLOAT:
case GLSL_TYPE_BOOL:
- case GLSL_TYPE_DOUBLE:
return this->matrix_columns;
+ case GLSL_TYPE_DOUBLE:
+ return this->matrix_columns * 2;
+
case GLSL_TYPE_STRUCT:
case GLSL_TYPE_INTERFACE: {
unsigned size = 0;
--
2.4.6
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] glsl: Properly count doubles as taking twice the number of attrib slots.
On Tue, Sep 1, 2015 at 3:57 PM, Matt Turner wrote: > --- > I checked the uses of count_attribute_slots() and it looks like they're > expecting this already, but these two patches definitely need testing on > a driver that supports fp64. Whoops, assuming things go well this should also be Cc'd to 10.6 and 11.0. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium: remove support for predicates from TGSI
I'm all for TGIS simplification, but I just checked, and we rely on TGSI predicates for some of our internal state trackers. So I'll need more time to evaluate how much effort it would be to not rely on it, and when can it be done. Jose On 01/09/15 22:19, Marek Olšák wrote: From: Marek Olšák Neved used. --- Take 2. Let's see how people feel about TGSI now. src/gallium/auxiliary/gallivm/lp_bld_limits.h | 4 - src/gallium/auxiliary/gallivm/lp_bld_tgsi.h| 2 - src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c| 46 --- src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c | 6 +- src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c| 138 ++- src/gallium/auxiliary/tgsi/tgsi_build.c| 66 - src/gallium/auxiliary/tgsi/tgsi_build.h| 3 - src/gallium/auxiliary/tgsi/tgsi_dump.c | 24 src/gallium/auxiliary/tgsi/tgsi_exec.c | 59 src/gallium/auxiliary/tgsi/tgsi_exec.h | 7 - src/gallium/auxiliary/tgsi/tgsi_parse.c| 4 - src/gallium/auxiliary/tgsi/tgsi_parse.h| 1 - src/gallium/auxiliary/tgsi/tgsi_sanity.c | 1 - src/gallium/auxiliary/tgsi/tgsi_strings.c | 1 - src/gallium/auxiliary/tgsi/tgsi_text.c | 37 - src/gallium/auxiliary/tgsi/tgsi_ureg.c | 84 +--- src/gallium/auxiliary/tgsi/tgsi_ureg.h | 149 + src/gallium/docs/source/screen.rst | 1 - src/gallium/drivers/freedreno/freedreno_screen.c | 2 - src/gallium/drivers/i915/i915_fpc.h| 1 - src/gallium/drivers/i915/i915_screen.c | 2 - src/gallium/drivers/ilo/ilo_screen.c | 2 - src/gallium/drivers/ilo/shader/toy_tgsi.c | 5 - .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 10 +- src/gallium/drivers/nouveau/nv30/nv30_screen.c | 2 - src/gallium/drivers/nouveau/nv50/nv50_screen.c | 2 - src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 2 - src/gallium/drivers/r300/r300_screen.c | 4 - src/gallium/drivers/r600/r600_pipe.c | 2 - src/gallium/drivers/r600/r600_shader.c | 4 - src/gallium/drivers/radeonsi/si_pipe.c | 2 - src/gallium/drivers/svga/svga_screen.c | 4 - src/gallium/include/pipe/p_defines.h | 1 - src/gallium/include/pipe/p_shader_tokens.h | 25 +--- src/gallium/state_trackers/nine/nine_shader.c | 18 +-- 35 files changed, 27 insertions(+), 694 deletions(-) diff --git a/src/gallium/auxiliary/gallivm/lp_bld_limits.h b/src/gallium/auxiliary/gallivm/lp_bld_limits.h index 571c615..da774bf 100644 --- a/src/gallium/auxiliary/gallivm/lp_bld_limits.h +++ b/src/gallium/auxiliary/gallivm/lp_bld_limits.h @@ -49,8 +49,6 @@ #define LP_MAX_TGSI_IMMEDIATES 4096 -#define LP_MAX_TGSI_PREDS 16 - #define LP_MAX_TGSI_CONSTS 4096 #define LP_MAX_TGSI_CONST_BUFFERS 16 @@ -109,8 +107,6 @@ gallivm_get_shader_param(enum pipe_shader_cap param) return PIPE_MAX_CONSTANT_BUFFERS; case PIPE_SHADER_CAP_MAX_TEMPS: return LP_MAX_TGSI_TEMPS; - case PIPE_SHADER_CAP_MAX_PREDS: - return LP_MAX_TGSI_PREDS; case PIPE_SHADER_CAP_TGSI_CONT_SUPPORTED: return 1; case PIPE_SHADER_CAP_INDIRECT_INPUT_ADDR: diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h index 2ca9c61..a48f008 100644 --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h @@ -455,7 +455,6 @@ struct lp_build_tgsi_soa_context LLVMValueRef immediates[LP_MAX_INLINED_IMMEDIATES][TGSI_NUM_CHANNELS]; LLVMValueRef temps[LP_MAX_INLINED_TEMPS][TGSI_NUM_CHANNELS]; LLVMValueRef addr[LP_MAX_TGSI_ADDRS][TGSI_NUM_CHANNELS]; - LLVMValueRef preds[LP_MAX_TGSI_PREDS][TGSI_NUM_CHANNELS]; /* We allocate/use this array of temps if (1 << TGSI_FILE_TEMPORARY) is * set in the indirect_files field. @@ -549,7 +548,6 @@ struct lp_build_tgsi_aos_context LLVMValueRef immediates[LP_MAX_INLINED_IMMEDIATES]; LLVMValueRef temps[LP_MAX_INLINED_TEMPS]; LLVMValueRef addr[LP_MAX_TGSI_ADDRS]; - LLVMValueRef preds[LP_MAX_TGSI_PREDS]; /* We allocate/use this array of temps if (1 << TGSI_FILE_TEMPORARY) is * set in the indirect_files field. diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c index 610283d..8094efc 100644 --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c @@ -256,10 +256,6 @@ lp_emit_store_aos( ptr = bld->addr[reg->Indirect.Index]; break; - case TGSI_FILE_PREDICATE: - ptr = bld->preds[reg->Register.Index]; - break; - default: assert(0); return; @@ -267,43 +263,6 @@ lp_emit_store_aos( if (!ptr
Re: [Mesa-dev] [PATCH 1/2] glsl: Properly count doubles as taking twice the number of attrib slots.
On Tue, Sep 1, 2015 at 6:53 PM, Matt Turner wrote: > On Tue, Sep 1, 2015 at 3:57 PM, Matt Turner wrote: >> --- >> I checked the uses of count_attribute_slots() and it looks like they're >> expecting this already, but these two patches definitely need testing on >> a driver that supports fp64. > > Whoops, assuming things go well this should also be Cc'd to 10.6 and 11.0. Sooo I can't tell whether your patch upsets this, but just want to make sure you're aware of this behaviour: Even though the driver is allowed to count doubles as double-wide on the back end for the purposes of max attributes, double vertex attributes are still only supposed to count as a single slot where bindings are concerned. I'll run these through a piglit -t fp64 though, hopefully that picks up any piglit tests that were supposed to catch this sort of thing. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] glsl: Properly count doubles as taking twice the number of attrib slots.
On 2 September 2015 at 08:57, Matt Turner wrote:
> ---
> I checked the uses of count_attribute_slots() and it looks like they're
> expecting this already, but these two patches definitely need testing on
> a driver that supports fp64.
>
Don't think so,
As Ilia said they can count as storage for 2, but only one location, I
believe this function
is used in enough place to count locations.
Dave.
> src/glsl/glsl_types.cpp | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 755618a..cd7fef5 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -1386,9 +1386,11 @@ glsl_type::count_attribute_slots() const
> case GLSL_TYPE_INT:
> case GLSL_TYPE_FLOAT:
> case GLSL_TYPE_BOOL:
> - case GLSL_TYPE_DOUBLE:
>return this->matrix_columns;
>
> + case GLSL_TYPE_DOUBLE:
> + return this->matrix_columns * 2;
> +
> case GLSL_TYPE_STRUCT:
> case GLSL_TYPE_INTERFACE: {
>unsigned size = 0;
> --
> 2.4.6
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] Revert "glsl: check total count of multi-slot double vertex attribs"
On 2 September 2015 at 08:57, Matt Turner wrote: > This reverts commit ad208d975a6d3aebe14f7c2c16039ee200d8b30c. > > This code attempted to ensure that we didn't use more than the maximum > number of attribute slots, but it did not consider attribute aliasing so > it would miscount slots used by aliased attributes and incorrectly > reject valid shaders. > > Worse, the code to handle attribute double slots was just working around > a mistake in count_attribute_slots() (Fixed by previous patch). that wasn't a mistake :-) Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa/api: validate buffers are unmapped
From: Dave Airlie
GL 3.2 introduced new language in this area, and CTS enforces it,
this patches checks all the vertex buffers and the index buffer
are mapped in the core profile case. I'm not sure what GLES
expects here.
This fixes
GL33-CTS.draw_elements_base_vertex_tests.invalid_mapped_bos
Signed-off-by: Dave Airlie
---
src/mesa/main/api_validate.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 53c8fb8..8b79d5c 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -34,7 +34,21 @@
#include "transformfeedback.h"
#include
+/* GL3.2 introduces checks for buffer mappings */
+static bool
+check_buffers_are_unmapped(struct gl_vertex_buffer_binding *inputs)
+{
+ GLuint i;
+ for (i = 0; i < VERT_ATTRIB_MAX; i++) {
+ if (inputs[i].BufferObj) {
+ struct gl_buffer_object *obj = inputs[i].BufferObj;
+ if (_mesa_check_disallowed_mapping(obj))
+return false;
+ }
+ }
+ return true;
+}
/**
* Check if OK to draw arrays/elements.
*/
@@ -58,6 +72,25 @@ check_valid_to_render(struct gl_context *ctx, const char
*function)
break;
case API_OPENGL_CORE:
+
+ /* Section 2.9.3 (Mapping and Unmapping Buffer Data" of the OpenGL 4.2
+ * Core Profile says:
+ *"Most, but not all GL commands will detect attempts to read data
+ * from a mapped buffer object. When such an attempt is detected, an
+ * INVALID_OPERATION error will be generated.
+ */
+ if (!check_buffers_are_unmapped(ctx->Array.VAO->VertexBinding)) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
function);
+ return false;
+ }
+
+ if (ctx->Array.VAO->IndexBufferObj) {
+ if (_mesa_check_disallowed_mapping(ctx->Array.VAO->IndexBufferObj)) {
+_mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
function);
+return false;
+ }
+ }
+
/* Section 10.4 (Drawing Commands Using Vertex Arrays) of the OpenGL 4.5
* Core Profile spec says:
*
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/api: validate buffers are unmapped
On Tue, Sep 1, 2015 at 7:39 PM, Dave Airlie wrote:
> From: Dave Airlie
>
> GL 3.2 introduced new language in this area, and CTS enforces it,
> this patches checks all the vertex buffers and the index buffer
> are mapped in the core profile case. I'm not sure what GLES
> expects here.
>
> This fixes
> GL33-CTS.draw_elements_base_vertex_tests.invalid_mapped_bos
>
> Signed-off-by: Dave Airlie
> ---
> src/mesa/main/api_validate.c | 33 +
> 1 file changed, 33 insertions(+)
>
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index 53c8fb8..8b79d5c 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -34,7 +34,21 @@
> #include "transformfeedback.h"
> #include
>
> +/* GL3.2 introduces checks for buffer mappings */
> +static bool
> +check_buffers_are_unmapped(struct gl_vertex_buffer_binding *inputs)
> +{
> + GLuint i;
>
> + for (i = 0; i < VERT_ATTRIB_MAX; i++) {
> + if (inputs[i].BufferObj) {
> + struct gl_buffer_object *obj = inputs[i].BufferObj;
> + if (_mesa_check_disallowed_mapping(obj))
> +return false;
> + }
> + }
> + return true;
> +}
> /**
> * Check if OK to draw arrays/elements.
> */
> @@ -58,6 +72,25 @@ check_valid_to_render(struct gl_context *ctx, const char
> *function)
>break;
>
> case API_OPENGL_CORE:
> +
> + /* Section 2.9.3 (Mapping and Unmapping Buffer Data" of the OpenGL 4.2
> + * Core Profile says:
> + *"Most, but not all GL commands will detect attempts to read data
> + * from a mapped buffer object. When such an attempt is detected,
> an
> + * INVALID_OPERATION error will be generated.
> + */
> + if (!check_buffers_are_unmapped(ctx->Array.VAO->VertexBinding)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
> function);
> + return false;
> + }
> +
> + if (ctx->Array.VAO->IndexBufferObj) {
> + if (_mesa_check_disallowed_mapping(ctx->Array.VAO->IndexBufferObj))
> {
> +_mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
> function);
May I suggest improving the messages to indicate that a vertex or
index buffer was being mapped? This should reduce confusion,
hopefully.
> +return false;
> + }
> + }
> +
>/* Section 10.4 (Drawing Commands Using Vertex Arrays) of the OpenGL
> 4.5
> * Core Profile spec says:
> *
> --
> 2.4.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/api: validate buffers are unmapped
On 2 September 2015 at 09:42, Ilia Mirkin wrote:
> On Tue, Sep 1, 2015 at 7:39 PM, Dave Airlie wrote:
>> From: Dave Airlie
>>
>> GL 3.2 introduced new language in this area, and CTS enforces it,
>> this patches checks all the vertex buffers and the index buffer
>> are mapped in the core profile case. I'm not sure what GLES
>> expects here.
>>
>> This fixes
>> GL33-CTS.draw_elements_base_vertex_tests.invalid_mapped_bos
>>
>> Signed-off-by: Dave Airlie
>> ---
>> src/mesa/main/api_validate.c | 33 +
>> 1 file changed, 33 insertions(+)
>>
>> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
>> index 53c8fb8..8b79d5c 100644
>> --- a/src/mesa/main/api_validate.c
>> +++ b/src/mesa/main/api_validate.c
>> @@ -34,7 +34,21 @@
>> #include "transformfeedback.h"
>> #include
>>
>> +/* GL3.2 introduces checks for buffer mappings */
>> +static bool
>> +check_buffers_are_unmapped(struct gl_vertex_buffer_binding *inputs)
>> +{
>> + GLuint i;
>>
>> + for (i = 0; i < VERT_ATTRIB_MAX; i++) {
>> + if (inputs[i].BufferObj) {
>> + struct gl_buffer_object *obj = inputs[i].BufferObj;
>> + if (_mesa_check_disallowed_mapping(obj))
>> +return false;
>> + }
>> + }
>> + return true;
>> +}
>> /**
>> * Check if OK to draw arrays/elements.
>> */
>> @@ -58,6 +72,25 @@ check_valid_to_render(struct gl_context *ctx, const char
>> *function)
>>break;
>>
>> case API_OPENGL_CORE:
>> +
>> + /* Section 2.9.3 (Mapping and Unmapping Buffer Data" of the OpenGL 4.2
>> + * Core Profile says:
>> + *"Most, but not all GL commands will detect attempts to read data
>> + * from a mapped buffer object. When such an attempt is detected,
>> an
>> + * INVALID_OPERATION error will be generated.
>> + */
>> + if (!check_buffers_are_unmapped(ctx->Array.VAO->VertexBinding)) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
>> function);
>> + return false;
>> + }
>> +
>> + if (ctx->Array.VAO->IndexBufferObj) {
>> + if
>> (_mesa_check_disallowed_mapping(ctx->Array.VAO->IndexBufferObj)) {
>> +_mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
>> function);
>
> May I suggest improving the messages to indicate that a vertex or
> index buffer was being mapped? This should reduce confusion,
> hopefully.
you mean someone is going to see this error message in the wild? I
admire your optimism :-)
But yes I spotted that myself, so I can do so.
Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/api: validate buffers are unmapped
On Tue, Sep 1, 2015 at 7:50 PM, Dave Airlie wrote:
> On 2 September 2015 at 09:42, Ilia Mirkin wrote:
>> On Tue, Sep 1, 2015 at 7:39 PM, Dave Airlie wrote:
>>> From: Dave Airlie
>>>
>>> GL 3.2 introduced new language in this area, and CTS enforces it,
>>> this patches checks all the vertex buffers and the index buffer
>>> are mapped in the core profile case. I'm not sure what GLES
>>> expects here.
>>>
>>> This fixes
>>> GL33-CTS.draw_elements_base_vertex_tests.invalid_mapped_bos
>>>
>>> Signed-off-by: Dave Airlie
>>> ---
>>> src/mesa/main/api_validate.c | 33 +
>>> 1 file changed, 33 insertions(+)
>>>
>>> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
>>> index 53c8fb8..8b79d5c 100644
>>> --- a/src/mesa/main/api_validate.c
>>> +++ b/src/mesa/main/api_validate.c
>>> @@ -34,7 +34,21 @@
>>> #include "transformfeedback.h"
>>> #include
>>>
>>> +/* GL3.2 introduces checks for buffer mappings */
>>> +static bool
>>> +check_buffers_are_unmapped(struct gl_vertex_buffer_binding *inputs)
>>> +{
>>> + GLuint i;
>>>
>>> + for (i = 0; i < VERT_ATTRIB_MAX; i++) {
>>> + if (inputs[i].BufferObj) {
>>> + struct gl_buffer_object *obj = inputs[i].BufferObj;
>>> + if (_mesa_check_disallowed_mapping(obj))
>>> +return false;
>>> + }
>>> + }
>>> + return true;
>>> +}
>>> /**
>>> * Check if OK to draw arrays/elements.
>>> */
>>> @@ -58,6 +72,25 @@ check_valid_to_render(struct gl_context *ctx, const char
>>> *function)
>>>break;
>>>
>>> case API_OPENGL_CORE:
>>> +
>>> + /* Section 2.9.3 (Mapping and Unmapping Buffer Data" of the OpenGL
>>> 4.2
>>> + * Core Profile says:
>>> + *"Most, but not all GL commands will detect attempts to read
>>> data
>>> + * from a mapped buffer object. When such an attempt is
>>> detected, an
>>> + * INVALID_OPERATION error will be generated.
>>> + */
>>> + if (!check_buffers_are_unmapped(ctx->Array.VAO->VertexBinding)) {
>>> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
>>> function);
>>> + return false;
>>> + }
>>> +
>>> + if (ctx->Array.VAO->IndexBufferObj) {
>>> + if
>>> (_mesa_check_disallowed_mapping(ctx->Array.VAO->IndexBufferObj)) {
>>> +_mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
>>> function);
>>
>> May I suggest improving the messages to indicate that a vertex or
>> index buffer was being mapped? This should reduce confusion,
>> hopefully.
>
> you mean someone is going to see this error message in the wild? I
> admire your optimism :-)
I mean I'm going to scratch my head less when I attempt to get a
piglit test working and the draws start failing randomly. (But don't
worry, there will still be a fair bit of head scratching in such an
event, so not all is lost.)
From what I can tell, the spec is unclear whether commands are
supposed to do the checking or not... hopefully someone with more spec
experience can comment.
>
> But yes I spotted that myself, so I can do so.
>
> Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/api: validate buffers are unmapped
On 09/01/2015 05:39 PM, Dave Airlie wrote:
From: Dave Airlie
GL 3.2 introduced new language in this area, and CTS enforces it,
this patches checks all the vertex buffers and the index buffer
are mapped in the core profile case. I'm not sure what GLES
s/mapped/unmapped/ ?
expects here.
This fixes
GL33-CTS.draw_elements_base_vertex_tests.invalid_mapped_bos
Signed-off-by: Dave Airlie
---
src/mesa/main/api_validate.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 53c8fb8..8b79d5c 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -34,7 +34,21 @@
#include "transformfeedback.h"
#include
+/* GL3.2 introduces checks for buffer mappings */
+static bool
+check_buffers_are_unmapped(struct gl_vertex_buffer_binding *inputs)
+{
+ GLuint i;
+ for (i = 0; i < VERT_ATTRIB_MAX; i++) {
+ if (inputs[i].BufferObj) {
+ struct gl_buffer_object *obj = inputs[i].BufferObj;
+ if (_mesa_check_disallowed_mapping(obj))
+return false;
+ }
+ }
+ return true;
+}
/**
* Check if OK to draw arrays/elements.
*/
@@ -58,6 +72,25 @@ check_valid_to_render(struct gl_context *ctx, const char
*function)
break;
case API_OPENGL_CORE:
+
+ /* Section 2.9.3 (Mapping and Unmapping Buffer Data" of the OpenGL 4.2
+ * Core Profile says:
+ *"Most, but not all GL commands will detect attempts to read data
+ * from a mapped buffer object. When such an attempt is detected, an
+ * INVALID_OPERATION error will be generated.
+ */
+ if (!check_buffers_are_unmapped(ctx->Array.VAO->VertexBinding)) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
function);
+ return false;
+ }
+
+ if (ctx->Array.VAO->IndexBufferObj) {
+ if (_mesa_check_disallowed_mapping(ctx->Array.VAO->IndexBufferObj)) {
+_mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
function);
+return false;
+ }
+ }
+
/* Section 10.4 (Drawing Commands Using Vertex Arrays) of the OpenGL 4.5
* Core Profile spec says:
*
There's some VBO code which checks that arrays are unmapped for
debugging purposes. Look for check_buffers_are_unmapped(). Maybe some
consolidation is possible.
-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/api: validate buffers are unmapped
On 2 September 2015 at 09:58, Brian Paul wrote:
> On 09/01/2015 05:39 PM, Dave Airlie wrote:
>>
>> From: Dave Airlie
>>
>> GL 3.2 introduced new language in this area, and CTS enforces it,
>> this patches checks all the vertex buffers and the index buffer
>> are mapped in the core profile case. I'm not sure what GLES
>
>
> s/mapped/unmapped/ ?
oops.
>
>
>
>> expects here.
>>
>> This fixes
>> GL33-CTS.draw_elements_base_vertex_tests.invalid_mapped_bos
>>
>> Signed-off-by: Dave Airlie
>> ---
>> src/mesa/main/api_validate.c | 33 +
>> 1 file changed, 33 insertions(+)
>>
>> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
>> index 53c8fb8..8b79d5c 100644
>> --- a/src/mesa/main/api_validate.c
>> +++ b/src/mesa/main/api_validate.c
>> @@ -34,7 +34,21 @@
>> #include "transformfeedback.h"
>> #include
>>
>> +/* GL3.2 introduces checks for buffer mappings */
>> +static bool
>> +check_buffers_are_unmapped(struct gl_vertex_buffer_binding *inputs)
>> +{
>> + GLuint i;
>>
>> + for (i = 0; i < VERT_ATTRIB_MAX; i++) {
>> + if (inputs[i].BufferObj) {
>> + struct gl_buffer_object *obj = inputs[i].BufferObj;
>> + if (_mesa_check_disallowed_mapping(obj))
>> +return false;
>> + }
>> + }
>> + return true;
>> +}
>> /**
>>* Check if OK to draw arrays/elements.
>>*/
>> @@ -58,6 +72,25 @@ check_valid_to_render(struct gl_context *ctx, const
>> char *function)
>> break;
>>
>> case API_OPENGL_CORE:
>> +
>> + /* Section 2.9.3 (Mapping and Unmapping Buffer Data" of the OpenGL
>> 4.2
>> + * Core Profile says:
>> + *"Most, but not all GL commands will detect attempts to read
>> data
>> + * from a mapped buffer object. When such an attempt is
>> detected, an
>> + * INVALID_OPERATION error will be generated.
>> + */
>> + if (!check_buffers_are_unmapped(ctx->Array.VAO->VertexBinding)) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
>> function);
>> + return false;
>> + }
>> +
>> + if (ctx->Array.VAO->IndexBufferObj) {
>> + if
>> (_mesa_check_disallowed_mapping(ctx->Array.VAO->IndexBufferObj)) {
>> +_mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)",
>> function);
>> +return false;
>> + }
>> + }
>> +
>> /* Section 10.4 (Drawing Commands Using Vertex Arrays) of the
>> OpenGL 4.5
>> * Core Profile spec says:
>> *
>>
>
> There's some VBO code which checks that arrays are unmapped for debugging
> purposes. Look for check_buffers_are_unmapped(). Maybe some consolidation
> is possible.
Yes I didn't want to touch vbo code, because it's too late in the
process to validate
stuff that that point, also it jsut asserts, not sure it helps to
merge it much, they both
use the _mesa_check_disallowed_mapping interface, which seems to be sharing
enough code.
Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] dri/common: embed drirc into driver binaries
From: Marek Olšák People are having issues with apps because drirc wasn't installed into /etc. I've lost patience. --- src/mesa/drivers/dri/common/Makefile.am | 4 +- src/mesa/drivers/dri/common/Makefile.sources | 3 +- src/mesa/drivers/dri/common/drirc| 84 src/mesa/drivers/dri/common/drirc_built_in.h | 111 +++ src/mesa/drivers/dri/common/xmlconfig.c | 30 +++- 5 files changed, 140 insertions(+), 92 deletions(-) delete mode 100644 src/mesa/drivers/dri/common/drirc create mode 100644 src/mesa/drivers/dri/common/drirc_built_in.h diff --git a/src/mesa/drivers/dri/common/Makefile.am b/src/mesa/drivers/dri/common/Makefile.am index b307f10..7106abe 100644 --- a/src/mesa/drivers/dri/common/Makefile.am +++ b/src/mesa/drivers/dri/common/Makefile.am @@ -23,7 +23,7 @@ SUBDIRS = xmlpool include Makefile.sources -EXTRA_DIST = drirc xmlpool.h SConscript +EXTRA_DIST = xmlpool.h SConscript AM_CFLAGS = \ -I$(top_srcdir)/include \ @@ -52,5 +52,3 @@ libdri_test_stubs_la_SOURCES = $(test_stubs_FILES) libdri_test_stubs_la_CFLAGS = $(AM_CFLAGS) -DNO_MAIN libmegadriver_stub_la_SOURCES = $(megadriver_stub_FILES) - -sysconf_DATA = drirc diff --git a/src/mesa/drivers/dri/common/Makefile.sources b/src/mesa/drivers/dri/common/Makefile.sources index d5d8da8..71ba01d 100644 --- a/src/mesa/drivers/dri/common/Makefile.sources +++ b/src/mesa/drivers/dri/common/Makefile.sources @@ -6,7 +6,8 @@ DRI_COMMON_FILES := \ XMLCONFIG_FILES := \ xmlconfig.c \ - xmlconfig.h + xmlconfig.h \ + drirc_built_in.h # Paths are relative to MESA_TOP. mesa_dri_common_INCLUDES := \ diff --git a/src/mesa/drivers/dri/common/drirc b/src/mesa/drivers/dri/common/drirc deleted file mode 100644 index bb840ea..000 --- a/src/mesa/drivers/dri/common/drirc +++ /dev/null @@ -1,84 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/mesa/drivers/dri/common/drirc_built_in.h b/src/mesa/drivers/dri/common/drirc_built_in.h new file mode 100644 index 000..592b1d1 --- /dev/null +++ b/src/mesa/drivers/dri/common/drirc_built_in.h @@ -0,0 +1,111 @@ +/* + * Copyright 2015 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * on the rights to use, copy, modify, merge, publish, distribute, sub + * license, and/or sell copies of the Software, and to permit persons to whom + * the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL + * THE AUTHOR(S) AND/OR THEIR SUPPLIERS BE LIABLE FOR ANY CLAIM, + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE + * USE OR OTHER DEALINGS IN THE SOFTWARE. + * + */ + +#ifndef DRIRC_BUILT_IN_H +#define DRIRC_BUILT_IN_H + +/* + +Application bugs worked around here: + + +* Unigine Heaven 3.0 and older contain too many bugs and can't be supported + by drivers that want to be compliant. + +* Various Unigine products don't use the #version and #extension GLSL + directives, meaning they only get GLSL 1.10 and no extensions for their + shaders. + Enabling all extensions for Unigine fixes most issues, but the GLSL version + is still 1.10. + +* If ARB_sample_shading is supported, Unigine Heaven 4.0 and Valley 1.0 uses + an #extension directive in the middle of its shaders, which is illegal + in GLSL. + +TODO: document the other workarounds. + +*/ + +static const char *drirc_built_in = +"" +"" +"" +"" +"" +"" + +"" +"" +"" +"" + +"" +"" +"" + +"" +"" +"" + +"" +"" +"" + +"" +"" +"" + +"" +"
[Mesa-dev] [PATCH 2/2] dri/common: drop loading /etc/drirc
From: Marek Olšák
A user can be using Mesa 11.0, but /etc/drirc can be from Mesa 10.5.
We don't want the old drirc to affect Mesa 11.0.
There are 2 options:
- use a different file name (e.g. /etc/drirc_global) for people wanting
a global drirc file, but they must supply it by themselves
- just don't load it, users should use ~/.drirc
This patch implements the latter.
---
src/mesa/drivers/dri/common/xmlconfig.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/mesa/drivers/dri/common/xmlconfig.c
b/src/mesa/drivers/dri/common/xmlconfig.c
index 47a9aef..a8f00ef 100644
--- a/src/mesa/drivers/dri/common/xmlconfig.c
+++ b/src/mesa/drivers/dri/common/xmlconfig.c
@@ -955,7 +955,7 @@ static void parseOneConfigFile (XML_Parser p) {
void driParseConfigFiles (driOptionCache *cache, const driOptionCache *info,
int screenNum, const char *driverName) {
-char *filenames[2] = {"/etc/drirc", NULL};
+char *filename = NULL;
char *home;
uint32_t i;
struct OptConfData userData;
@@ -969,25 +969,25 @@ void driParseConfigFiles (driOptionCache *cache, const
driOptionCache *info,
if ((home = getenv ("HOME"))) {
uint32_t len = strlen (home);
- filenames[1] = malloc(len + 7+1);
- if (filenames[1] == NULL)
+ filename = malloc(len + 7+1);
+ if (filename == NULL)
__driUtilMessage ("Can't allocate memory for %s/.drirc.", home);
else {
- memcpy (filenames[1], home, len);
- memcpy (filenames[1] + len, "/.drirc", 7+1);
+ memcpy (filename, home, len);
+ memcpy (filename + len, "/.drirc", 7+1);
}
}
-for (i = 0; i < 3; ++i) {
+for (i = 0; i < 2; ++i) {
XML_Parser p;
- if (i && filenames[i-1] == NULL)
+ if (i && filename == NULL)
continue;
p = XML_ParserCreate (NULL); /* use encoding specified by file */
XML_SetElementHandler (p, optConfStartElem, optConfEndElem);
XML_SetUserData (p, &userData);
userData.parser = p;
- userData.name = i ? filenames[i-1] : NULL;
+ userData.name = i ? filename : NULL;
userData.ignoringDevice = 0;
userData.ignoringApp = 0;
userData.inDriConf = 0;
@@ -1003,7 +1003,7 @@ void driParseConfigFiles (driOptionCache *cache, const
driOptionCache *info,
XML_ParserFree (p);
}
-free(filenames[1]);
+free(filename);
}
void driDestroyOptionInfo (driOptionCache *info) {
--
2.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] dri/common: embed drirc into driver binaries
I was actually going to suggest loading from the sysconfig dir instead of /etc a while back, but this works for me too. I haven't looked over the details, but the idea of this series is Acked-by: Ilia Mirkin On Tue, Sep 1, 2015 at 8:26 PM, Marek Olšák wrote: > From: Marek Olšák > > People are having issues with apps because drirc wasn't installed > into /etc. I've lost patience. > --- > src/mesa/drivers/dri/common/Makefile.am | 4 +- > src/mesa/drivers/dri/common/Makefile.sources | 3 +- > src/mesa/drivers/dri/common/drirc| 84 > src/mesa/drivers/dri/common/drirc_built_in.h | 111 > +++ > src/mesa/drivers/dri/common/xmlconfig.c | 30 +++- > 5 files changed, 140 insertions(+), 92 deletions(-) > delete mode 100644 src/mesa/drivers/dri/common/drirc > create mode 100644 src/mesa/drivers/dri/common/drirc_built_in.h > > diff --git a/src/mesa/drivers/dri/common/Makefile.am > b/src/mesa/drivers/dri/common/Makefile.am > index b307f10..7106abe 100644 > --- a/src/mesa/drivers/dri/common/Makefile.am > +++ b/src/mesa/drivers/dri/common/Makefile.am > @@ -23,7 +23,7 @@ SUBDIRS = xmlpool > > include Makefile.sources > > -EXTRA_DIST = drirc xmlpool.h SConscript > +EXTRA_DIST = xmlpool.h SConscript > > AM_CFLAGS = \ > -I$(top_srcdir)/include \ > @@ -52,5 +52,3 @@ libdri_test_stubs_la_SOURCES = $(test_stubs_FILES) > libdri_test_stubs_la_CFLAGS = $(AM_CFLAGS) -DNO_MAIN > > libmegadriver_stub_la_SOURCES = $(megadriver_stub_FILES) > - > -sysconf_DATA = drirc > diff --git a/src/mesa/drivers/dri/common/Makefile.sources > b/src/mesa/drivers/dri/common/Makefile.sources > index d5d8da8..71ba01d 100644 > --- a/src/mesa/drivers/dri/common/Makefile.sources > +++ b/src/mesa/drivers/dri/common/Makefile.sources > @@ -6,7 +6,8 @@ DRI_COMMON_FILES := \ > > XMLCONFIG_FILES := \ > xmlconfig.c \ > - xmlconfig.h > + xmlconfig.h \ > + drirc_built_in.h > > # Paths are relative to MESA_TOP. > mesa_dri_common_INCLUDES := \ > diff --git a/src/mesa/drivers/dri/common/drirc > b/src/mesa/drivers/dri/common/drirc > deleted file mode 100644 > index bb840ea..000 > --- a/src/mesa/drivers/dri/common/drirc > +++ /dev/null > @@ -1,84 +0,0 @@ > - > - > - > - > - > - > - > - > - > - > - > - > - > - > - > - > - value="true" /> > - > - > - > - value="true" /> > - > - > - > - value="true" /> > - > - > - > - value="true" /> > - > - > - executable="OilRush_x86"> > - > - value="true" /> > - > - > - executable="OilRush_x64"> > - > - value="true" /> > - > - > - > - > - > - > - > - > - > - > - > - > - > - > - > - value="true" /> > - > - > - executable="do-not-directly-run-secondlife-bin"> > - value="true" /> > - > - > - > diff --git a/src/mesa/drivers/dri/common/drirc_built_in.h > b/src/mesa/drivers/dri/common/drirc_built_in.h > new file mode 100644 > index 000..592b1d1 > --- /dev/null > +++ b/src/mesa/drivers/dri/common/drirc_built_in.h > @@ -0,0 +1,111 @@ > +/* > + * Copyright 2015 Advanced Micro Devices, Inc. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * on the rights to use, copy, modify, merge, publish, distribute, sub > + * license, and/or sell copies of the Software, and to permit persons to whom > + * the Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHOR(S) AND/OR THEIR SUPPLIERS BE LIABLE FOR ANY CLAIM, > + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE > + * USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + */ > + > +#ifndef DRIRC_BUILT_IN_H > +#define DRIRC_BUILT_IN_H > + > +/* > + > +Application bugs worked around here: > + > + > +* Unigine Heaven 3.0 and older contain too many bugs and can't be supported > + by drivers that want to be compliant. > + > +* Various Unigin
[Mesa-dev] [PATCH 13/17 v2] glsl: Silence unused parameter warnings
From: Ian Romanick
builtin_variables.cpp:1062:53: warning: unused parameter 'name_as_gs_input'
[-Wunused-parameter]
const char *name_as_gs_input)
^
builtin_functions.cpp:4774:47: warning: unused parameter 'intrinsic_name'
[-Wunused-parameter]
const char *intrinsic_name,
^
builtin_functions.cpp:4907:66: warning: unused parameter 'state'
[-Wunused-parameter]
_mesa_glsl_find_builtin_function_by_name(_mesa_glsl_parse_state *state,
^
builtin_functions.cpp:4915:49: warning: unused parameter 'num_arguments'
[-Wunused-parameter]
unsigned num_arguments,
^
builtin_functions.cpp:4916:49: warning: unused parameter 'flags'
[-Wunused-parameter]
unsigned flags)
^
ir_print_visitor.cpp:589:37: warning: unused parameter 'ir' [-Wunused-parameter]
ir_print_visitor::visit(ir_barrier *ir)
^
linker.cpp:3212:48: warning: unused parameter 'ctx' [-Wunused-parameter]
build_program_resource_list(struct gl_context *ctx,
^
standalone_scaffolding.cpp:65:57: warning: unused parameter ‘id’
[-Wunused-parameter]
_mesa_shader_debug(struct gl_context *, GLenum, GLuint *id,
^
Now src/glsl is _almost_ free of warnings!
v2: Rebase on top of GL_ARB_shader_image_size work (especially
58a86897). Silence more warnings added by that work.
Signed-off-by: Ian Romanick
Reviewed-by: Ilia Mirkin [v1]
Cc: "Martin Peres "
---
src/glsl/ast_to_hir.cpp | 2 +-
src/glsl/builtin_functions.cpp | 14 --
src/glsl/builtin_variables.cpp | 8 +++-
src/glsl/ir.h | 3 +--
src/glsl/ir_print_visitor.cpp | 2 +-
src/glsl/linker.cpp | 3 +--
src/glsl/program.h | 3 +--
src/glsl/standalone_scaffolding.cpp | 2 +-
src/mesa/program/ir_to_mesa.cpp | 2 +-
9 files changed, 14 insertions(+), 25 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 517841c..72c6459 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -4578,7 +4578,7 @@ ast_function::hir(exec_list *instructions,
if (state->es_shader && state->language_version >= 300) {
/* Local shader has no exact candidates; check the built-ins. */
_mesa_glsl_initialize_builtin_functions();
- if (_mesa_glsl_find_builtin_function_by_name(state, name)) {
+ if (_mesa_glsl_find_builtin_function_by_name(name)) {
YYLTYPE loc = this->get_location();
_mesa_glsl_error(& loc, state,
"A shader cannot redefine or overload built-in "
diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
index 5e05199..3b4a9df 100644
--- a/src/glsl/builtin_functions.cpp
+++ b/src/glsl/builtin_functions.cpp
@@ -522,7 +522,6 @@ private:
void add_function(const char *name, ...);
typedef ir_function_signature
*(builtin_builder::*image_prototype_ctr)(const glsl_type *image_type,
-
const char *intrinsic_name,
unsigned num_arguments,
unsigned flags);
@@ -738,11 +737,9 @@ private:
B1(mid3)
ir_function_signature *_image_prototype(const glsl_type *image_type,
- const char *intrinsic_name,
unsigned num_arguments,
unsigned flags);
ir_function_signature *_image_size_prototype(const glsl_type *image_type,
-const char *intrinsic_name,
unsigned num_arguments,
unsigned flags);
ir_function_signature *_image(image_prototype_ctr prototype,
@@ -4866,7 +4863,6 @@ builtin_builder::_mid3(const glsl_type *type)
ir_function_signature *
builtin_builder::_image_prototype(const glsl_type *image_type,
- const char *intrinsic_name,
unsigned num_arguments,
unsigned flags)
{
@@ -4916,9 +4912,8 @@ builtin_builder::_image_prototype(const glsl_type
*image_type,
ir_function_signature *
builtin_builder::_image_size_prototype(const glsl_type *image_type,
- const char *intrinsic_name,
-
[Mesa-dev] [PATCH V3 2/6] glsl: assign hidden uniforms their slot id earlier
This is required so that the next patch can safely assign the slot id
to the var.
The ids are now assigned in the order we want before allocating storage
so there is no need to sort the storage array and move things around.
---
src/glsl/link_uniforms.cpp | 90 +-
1 file changed, 41 insertions(+), 49 deletions(-)
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 5402c99..da8f2f7 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -300,11 +300,12 @@ namespace {
*/
class count_uniform_size : public program_resource_visitor {
public:
- count_uniform_size(struct string_to_uint_map *map)
- : num_active_uniforms(0), num_values(0), num_shader_samplers(0),
-num_shader_images(0), num_shader_uniform_components(0),
-num_shader_subroutines(0),
-is_ubo_var(false), map(map)
+ count_uniform_size(struct string_to_uint_map *map,
+ struct string_to_uint_map *hidden_map)
+ : num_active_uniforms(0), num_hidden_uniforms(0), num_values(0),
+num_shader_samplers(0), num_shader_images(0),
+num_shader_uniform_components(0), num_shader_subroutines(0),
+is_ubo_var(false), map(map), hidden_map(hidden_map)
{
/* empty */
}
@@ -319,6 +320,7 @@ public:
void process(ir_variable *var)
{
+ this->current_var = var;
this->is_ubo_var = var->is_in_buffer_block();
if (var->is_interface_instance())
program_resource_visitor::process(var->get_interface_type(),
@@ -332,6 +334,8 @@ public:
*/
unsigned num_active_uniforms;
+ unsigned num_hidden_uniforms;
+
/**
* Number of data values required to back the storage for the active
uniforms
*/
@@ -359,6 +363,8 @@ public:
bool is_ubo_var;
+ struct string_to_uint_map *map;
+
private:
virtual void visit_field(const glsl_type *type, const char *name,
bool row_major)
@@ -402,7 +408,13 @@ private:
if (this->map->get(id, name))
return;
- this->map->put(this->num_active_uniforms, name);
+ if (this->current_var->data.how_declared == ir_var_hidden) {
+ this->hidden_map->put(this->num_hidden_uniforms, name);
+ this->num_hidden_uniforms++;
+ } else {
+ this->map->put(this->num_active_uniforms - this->num_hidden_uniforms,
+name);
+ }
/* Each leaf uniform occupies one entry in the list of active
* uniforms.
@@ -411,7 +423,12 @@ private:
this->num_values += values;
}
- struct string_to_uint_map *map;
+ struct string_to_uint_map *hidden_map;
+
+ /**
+* Current variable being processed.
+*/
+ ir_variable *current_var;
};
} /* anonymous namespace */
@@ -961,47 +978,19 @@ link_set_image_access_qualifiers(struct gl_shader_program
*prog)
}
/**
- * Sort the array of uniform storage so that the non-hidden uniforms are first
- *
- * This function sorts the list "in place." This is important because some of
- * the storage accessible from \c uniforms has \c uniforms as its \c ralloc
- * context. If \c uniforms is freed, some other storage will also be freed.
+ * Combine the hidden uniform hash map with the uniform hash map so that the
+ * hidden uniforms will be given indicies at the end of the uniform storage
+ * array.
*/
-static unsigned
-move_hidden_uniforms_to_end(struct gl_shader_program *prog,
-struct gl_uniform_storage *uniforms,
-unsigned num_elements)
+static void
+assign_hidden_uniform_slot_id(const char *name, unsigned hidden_id,
+ void *closure)
{
- struct gl_uniform_storage *sorted_uniforms =
- ralloc_array(prog, struct gl_uniform_storage, num_elements);
- unsigned hidden_uniforms = 0;
- unsigned j = 0;
-
- /* Add the non-hidden uniforms. */
- for (unsigned i = 0; i < num_elements; i++) {
- if (!uniforms[i].hidden)
- sorted_uniforms[j++] = uniforms[i];
- }
-
- /* Add and count the hidden uniforms. */
- for (unsigned i = 0; i < num_elements; i++) {
- if (uniforms[i].hidden) {
- sorted_uniforms[j++] = uniforms[i];
- hidden_uniforms++;
- }
- }
+ count_uniform_size *uniform_size = (count_uniform_size *) closure;
+ unsigned hidden_slot_id = uniform_size->num_active_uniforms -
+ uniform_size->num_hidden_uniforms + hidden_id;
- assert(prog->UniformHash != NULL);
- prog->UniformHash->clear();
- for (unsigned i = 0; i < num_elements; i++) {
- if (sorted_uniforms[i].name != NULL)
- prog->UniformHash->put(i, sorted_uniforms[i].name);
- }
-
- memcpy(uniforms, sorted_uniforms, sizeof(uniforms[0]) * num_elements);
- ralloc_free(sorted_uniforms);
-
- return hidden_uniforms;
+ uniform_size->map->put(hidden_slot_id, name);
}
void
@@ -1025,7 +1014,8 @@ link_assign_uniform_locations(struct gl_shader_prog
[Mesa-dev] [PATCH V3 1/6] glsl: order indices for samplers inside a struct array
This allows the correct offset to be easily calculated for indirect
indexing when a struct array contains multiple samplers, or any crazy
nesting.
The indices for the folling struct will now look like this:
Sampler index: 0 Name: s[0].tex
Sampler index: 1 Name: s[1].tex
Sampler index: 2 Name: s[0].si.tex
Sampler index: 3 Name: s[1].si.tex
Sampler index: 4 Name: s[0].si.tex2
Sampler index: 5 Name: s[1].si.tex2
Before this change it looked like this:
Sampler index: 0 Name: s[0].tex
Sampler index: 3 Name: s[1].tex
Sampler index: 1 Name: s[0].si.tex
Sampler index: 4 Name: s[1].si.tex
Sampler index: 2 Name: s[0].si.tex2
Sampler index: 5 Name: s[1].si.tex2
struct S_inner {
sampler2D tex;
sampler2D tex2;
};
struct S {
sampler2D tex;
S_inner si;
};
uniform S s[2];
V2: rename struct array counter to have better name
---
src/glsl/link_uniforms.cpp | 112 ++---
src/glsl/linker.h | 4 +-
2 files changed, 98 insertions(+), 18 deletions(-)
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 254086d..5402c99 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -28,6 +28,7 @@
#include "glsl_symbol_table.h"
#include "program/hash_table.h"
#include "program.h"
+#include "util/hash_table.h"
/**
* \file link_uniforms.cpp
@@ -63,14 +64,17 @@ program_resource_visitor::process(const glsl_type *type,
const char *name)
assert(type->without_array()->is_record()
|| type->without_array()->is_interface());
+ unsigned record_array_count = 1;
char *name_copy = ralloc_strdup(NULL, name);
- recursion(type, &name_copy, strlen(name), false, NULL, false);
+ recursion(type, &name_copy, strlen(name), false, NULL, false,
+ record_array_count);
ralloc_free(name_copy);
}
void
program_resource_visitor::process(ir_variable *var)
{
+ unsigned record_array_count = 1;
const glsl_type *t = var->type;
const bool row_major =
var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
@@ -111,7 +115,8 @@ program_resource_visitor::process(ir_variable *var)
* lowering is only applied to non-uniform interface blocks, so we
* can safely pass false for row_major.
*/
- recursion(var->type, &name, new_length, row_major, NULL, false);
+ recursion(var->type, &name, new_length, row_major, NULL, false,
+ record_array_count);
}
ralloc_free(name);
} else if (var->data.from_named_ifc_block_nonarray) {
@@ -135,19 +140,23 @@ program_resource_visitor::process(ir_variable *var)
* is only applied to non-uniform interface blocks, so we can safely
* pass false for row_major.
*/
- recursion(var->type, &name, strlen(name), row_major, NULL, false);
+ recursion(var->type, &name, strlen(name), row_major, NULL, false,
+record_array_count);
ralloc_free(name);
} else if (t->without_array()->is_record()) {
char *name = ralloc_strdup(NULL, var->name);
- recursion(var->type, &name, strlen(name), row_major, NULL, false);
+ recursion(var->type, &name, strlen(name), row_major, NULL, false,
+record_array_count);
ralloc_free(name);
} else if (t->is_interface()) {
char *name = ralloc_strdup(NULL, var->type->name);
- recursion(var->type, &name, strlen(name), row_major, NULL, false);
+ recursion(var->type, &name, strlen(name), row_major, NULL, false,
+record_array_count);
ralloc_free(name);
} else if (t->is_array() && t->fields.array->is_interface()) {
char *name = ralloc_strdup(NULL, var->type->fields.array->name);
- recursion(var->type, &name, strlen(name), row_major, NULL, false);
+ recursion(var->type, &name, strlen(name), row_major, NULL, false,
+record_array_count);
ralloc_free(name);
} else {
this->visit_field(t, var->name, row_major, NULL, false);
@@ -158,7 +167,8 @@ void
program_resource_visitor::recursion(const glsl_type *t, char **name,
size_t name_length, bool row_major,
const glsl_type *record_type,
-bool last_field)
+bool last_field,
+unsigned record_array_count)
{
/* Records need to have each field processed individually.
*
@@ -205,7 +215,7 @@ program_resource_visitor::recursion(const glsl_type *t,
char **name,
recursion(t->fields.structure[i].type, name, new_length,
field_row_major,
record_type,
- (i + 1) == t->length);
+ (i + 1) == t->length, record_array_count);
/* Only the first leaf-field of the record gets called with the
* record type pointer.
@@ -222,6 +232,8 @@ program_resource_visitor::recursion(const glsl_t
