Re: [Mesa-dev] [PATCH 1/2] clover: fix tab/space and alignement
On Monday 10 November 2014 19:03:43 Francisco Jerez wrote:
> EdB writes:
> > ---
> >
> > src/gallium/state_trackers/clover/llvm/invocation.cpp | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > b/src/gallium/state_trackers/clover/llvm/invocation.cpp index
> > 3a4fcf0..d29f5a6 100644
> > --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > @@ -214,7 +214,7 @@ namespace {
> >
> > #if HAVE_LLVM >= 0x0306
> >
> >c.getPreprocessorOpts().addRemappedFile(name,
> >
> > -
> > llvm::MemoryBuffer::getMemBuffer(source).release()); +
> > llvm::MemoryBuffer::getMemBuffer(source).release());
> This doesn't to fix the inconsistency that the line goes over the 80
> column limit, if that's what you meant to fix, and it makes the
> alignment with the open parenthesis inconsistent.
I tried to make it consistent with the bottom line.
Please use the style that fit better
EdB
>
> > #else
> >
> >c.getPreprocessorOpts().addRemappedFile(name,
> >
> >llvm::MemoryBuffer::getMemBuffer(so
> >urce));
> >
> > @@ -675,7 +675,7 @@ static const struct debug_named_value debug_options[]
> > = {>
> > {"llvm", DBG_LLVM, "Dump the generated LLVM IR for all kernels."},
> > {"asm", DBG_ASM, "Dump kernel assembly code for targets specifying "
> >
> > "PIPE_SHADER_IR_NATIVE"},
> >
> > - DEBUG_NAMED_VALUE_END // must be last
> > + DEBUG_NAMED_VALUE_END // must be last
> >
> > };
> >
> > module
> >
> > @@ -737,7 +737,7 @@ clover::compile_program_llvm(const compat::string
> > &source,>
> > break;
> >
> >case PIPE_SHADER_IR_NATIVE: {
> >
> > std::vector code = compile_native(mod, triple, processor,
> >
> > -debug_flags & DBG_ASM,
r_log);
> > + debug_flags & DBG_ASM,
> > r_log);>
> > m = build_module_native(code, mod, kernels, address_spaces,
> > r_log);
> > break;
> >
> >}
> >
> > ___
> > 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 1/2] i965/fs: Don't use if_block/else_block in SEL peephole.
---
src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp
b/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp
index c3bfd00..f18589c 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp
@@ -132,16 +132,21 @@ fs_visitor::opt_peephole_sel()
if (if_inst->opcode != BRW_OPCODE_IF)
continue;
- if (!block->else_block)
- continue;
-
- assert(block->else_block->end()->opcode == BRW_OPCODE_ELSE);
-
fs_inst *else_mov[MAX_MOVS] = { NULL };
fs_inst *then_mov[MAX_MOVS] = { NULL };
bblock_t *then_block = block->next();
- bblock_t *else_block = block->else_block->next();
+ bblock_t *else_block = NULL;
+ foreach_list_typed(bblock_link, child, link, &block->children) {
+ if (child->block != then_block) {
+if (child->block->prev()->end()->opcode == BRW_OPCODE_ELSE) {
+ else_block = child->block;
+}
+break;
+ }
+ }
+ if (else_block == NULL)
+ continue;
int movs = count_movs_from_if(then_mov, else_mov, then_block,
else_block);
--
2.0.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] i965/cfg: Remove if_block/else_block.
I used these in the SEL peephole, but they require extra tracking and
fix ups. The SEL peephole can pretty easily find the blocks it needs
without these.
---
src/mesa/drivers/dri/i965/brw_cfg.cpp | 15 +--
src/mesa/drivers/dri/i965/brw_cfg.h | 8
src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp | 8
3 files changed, 1 insertion(+), 30 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_cfg.cpp
b/src/mesa/drivers/dri/i965/brw_cfg.cpp
index bb49a0a..02149e2 100644
--- a/src/mesa/drivers/dri/i965/brw_cfg.cpp
+++ b/src/mesa/drivers/dri/i965/brw_cfg.cpp
@@ -51,8 +51,7 @@ link(void *mem_ctx, bblock_t *block)
}
bblock_t::bblock_t(cfg_t *cfg) :
- cfg(cfg), start_ip(0), end_ip(0), num(0),
- if_block(NULL), else_block(NULL)
+ cfg(cfg), start_ip(0), end_ip(0), num(0)
{
instructions.make_empty();
parents.make_empty();
@@ -136,7 +135,6 @@ bblock_t::combine_with(bblock_t *that)
}
this->end_ip = that->end_ip;
- this->else_block = that->else_block;
this->instructions.append_list(&that->instructions);
this->cfg->remove_block(that);
@@ -238,17 +236,6 @@ cfg_t::cfg_t(exec_list *instructions)
assert(cur_if->end()->opcode == BRW_OPCODE_IF);
assert(!cur_else || cur_else->end()->opcode == BRW_OPCODE_ELSE);
- cur_if->if_block = cur_if;
- cur_if->else_block = cur_else;
-
-if (cur_else) {
-cur_else->if_block = cur_if;
-cur_else->else_block = cur_else;
- }
-
- cur->if_block = cur_if;
- cur->else_block = cur_else;
-
/* Pop the stack so we're in the previous if/else/endif */
cur_if = pop_stack(&if_stack);
cur_else = pop_stack(&else_stack);
diff --git a/src/mesa/drivers/dri/i965/brw_cfg.h
b/src/mesa/drivers/dri/i965/brw_cfg.h
index 388d29e..48bca9f 100644
--- a/src/mesa/drivers/dri/i965/brw_cfg.h
+++ b/src/mesa/drivers/dri/i965/brw_cfg.h
@@ -89,14 +89,6 @@ struct bblock_t {
struct exec_list parents;
struct exec_list children;
int num;
-
- /* If the current basic block ends in an IF or ELSE instruction, these will
-* point to the basic blocks containing the other associated instruction.
-*
-* Otherwise they are NULL.
-*/
- struct bblock_t *if_block;
- struct bblock_t *else_block;
};
static inline struct backend_instruction *
diff --git a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
index 4c9d7b9..03f838d 100644
--- a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
+++ b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
@@ -85,7 +85,6 @@ dead_control_flow_eliminate(backend_visitor *v)
}
if (else_inst) {
-else_block->if_block->else_block = NULL;
else_inst->remove(else_block);
}
@@ -102,13 +101,6 @@ dead_control_flow_eliminate(backend_visitor *v)
if (earlier_block && earlier_block->can_combine_with(later_block)) {
earlier_block->combine_with(later_block);
-foreach_block (block, v->cfg) {
- if (block->if_block == later_block)
- block->if_block = earlier_block;
- if (block->else_block == later_block)
- block->else_block = earlier_block;
-}
-
/* If ENDIF was in its own block, then we've now deleted it and
* merged the two surrounding blocks, the latter of which the
* __next block pointer was pointing to.
--
2.0.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Use the predicate enable bit for conditional rendering without stalling
On Monday, November 10, 2014 02:57:22 PM Neil Roberts wrote: > I'm not really sure if I understand your reply correctly. Did you see > that I also posted the corresponding kernel patch? I was testing my > patch on top of the drm-intel-next branch with IvyBridge. That's why. Ivybridge has had the hardware checker disabled for a long time (basically, enabling PPGTT disables it), so you could write any registers you want. Haswell allows PPGTT + the hardware checker, so the kernel does that, and your register writes git turned into MI_NOOP. drm-intel-next must have the new software checker turned on, which disallows non-whitelisted register writes (along with libva, so it can't really be enabled upstream yet). --Ken 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] How difficult would it be to have debugging information for Jitted code show up?
On Tuesday, November 11, 2014 12:27:26 AM Steven Stewart-Gallus wrote: > Hello, > > Before filing a bug report, I wanted to have ask first how difficult it'd be > to > have debugging information for Jitted code show up be. It occurs to me that > this > probably wouldn't work for precompiled shader sources and that there's > probably > not a lot of code that has been written to support this. However, LLVM > obviously > has support for debugging information and as Mesa relies upon LLVM maybe most > of > the hardwork has already been done? I would guess that this would be easiest > for > software rendering and maybe possible, maybe not possible for the code that > uses > the Intel 3D hardware abilities? > > Thank you, > Steven Stewart-Gallus What kind of debugging information are you looking for? We have a few environment variables already... INTEL_DEBUG=perf Prints warnings when having to recompile shaders based on GL state; also warns when draw-time recompiling stalls the GPU INTEL_DEBUG=vs,fs,gs Prints out shader assembly, including the number of instructions and loops MESA_GLSL=dump Prints out shader source and compiler intermediate representation With the Intel driver, we have some tools that can measure instruction counts across a collection of shaders, and compare between reports: http://cgit.freedesktop.org/mesa/shader-db FWIW, LLVM is not used on Intel, nor (AFAIK) nouveau. radeonsi and llvmpipe both require it, and I believe it's optional for r600. We could certainly add more information, if there's something you're looking for and not able to find today. We've been adding things as we've found a need for them... --Ken 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] i965: Fix segfault in WebGL Conformance on Ivybridge
The patch seems to fix the crash I used to see while running this
WebGL conformance test on my machine (Intel(R) Core(TM) i7-4600U CPU @
2.10GHz). and now I can see all the tests pass. I believe 78770 is a
different issue (Added my comments to 78770) and not actually dealing
with fixing the crash we saw on 64 bit machines.
Br,
Kalyan
On Mon, Nov 10, 2014 at 8:02 AM, Chad Versace wrote:
> Fixes regression of WebGL Conformance test texture-size-limit [1] on
> Ivybridge Mobile GT2 0x0166 with Google Chrome R38.
>
> Regression introduced by
>
> commit 6c044231535b93c5d16404528946cad618d96bd9
> Author: Kenneth Graunke
> Date: Sun Feb 2 02:58:42 2014 -0800
>
> i965: Bump GL_MAX_CUBE_MAP_TEXTURE_SIZE to 8192.
>
> The test regressed because the pointer offset arithmetic in
> intel_miptree_map_gtt() overflows for large textures. The pointer
> arithmetic is not 64-bit safe.
>
> This patch fixes the bugzilla ticket below on Ivybridge. This patch
> doesn't close the ticket, though, because the bug report is against
> Sandybridge, and QA cannot confirm the fix on that hardware.
>
> [1]
> https://github.com/KhronosGroup/WebGL/blob/52f0dc240f04dce31b1b8e2b8107fe2b8332dc90/sdk/tests/conformance/textures/texture-size-limit.html
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78770
> Fixes: Intel CHRMOS-1377
> Reported-by: Lu Hua
> Signed-off-by: Chad Versace
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 8fda25d..24e217c 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1769,7 +1769,16 @@ intel_miptree_map_gtt(struct brw_context *brw,
>y += image_y;
>
>map->stride = mt->pitch;
> - map->ptr = base + y * map->stride + x * mt->cpp;
> +
> + /* The variables in below pointer arithmetic are 32-bit. The arithmetic
> + * overflows for large textures. Therefore the cast to intptr_t is
> + * needed.
> + *
> + * TODO(chadv): Fix this everywhere in i965 by fixing the signature of
> + * intel_miptree_get_image_offset() to use intptr_t.
> + */
> + map->ptr = base + (intptr_t) y * (intptr_t) map->stride
> + + (intptr_t) x * (intptr_t) mt->cpp;
> }
>
> DBG("%s: %d,%d %dx%d from mt %p (%s) %d,%d = %p/%d\n", __FUNCTION__,
> --
> 2.1.0-rc0
>
> ___
> 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] How difficult would it be to have debugging information for Jitted code show up?
Hello, Before filing a bug report, I wanted to have ask first how difficult it'd be to have debugging information for Jitted code show up be. It occurs to me that this probably wouldn't work for precompiled shader sources and that there's probably not a lot of code that has been written to support this. However, LLVM obviously has support for debugging information and as Mesa relies upon LLVM maybe most of the hardwork has already been done? I would guess that this would be easiest for software rendering and maybe possible, maybe not possible for the code that uses the Intel 3D hardware abilities? Thank you, Steven Stewart-Gallus ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 04/14] i965/gen7: Don't allocate hiz miptree structure
On Mon, Jul 21, 2014 at 11:00:53PM -0700, Jordan Justen wrote:
> We now skip allocating a hiz miptree for gen7. Instead, we calculate
> the required hiz buffer parameters and allocate a bo directly.
>
> v2:
> * Update hz_height calculation as suggested by Topi
>
> Signed-off-by: Jordan Justen
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 97
> ++-
> 1 file changed, 95 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index d5a2227..e3bcd8a 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -823,7 +823,10 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
>drm_intel_bo_unreference((*mt)->bo);
>intel_miptree_release(&(*mt)->stencil_mt);
>if ((*mt)->hiz_buf) {
> - intel_miptree_release(&(*mt)->hiz_buf->mt);
> + if ((*mt)->hiz_buf->mt)
> +intel_miptree_release(&(*mt)->hiz_buf->mt);
> + else
> +drm_intel_bo_unreference((*mt)->hiz_buf->bo);
> free((*mt)->hiz_buf);
>}
>intel_miptree_release(&(*mt)->mcs_mt);
> @@ -1374,6 +1377,91 @@ intel_miptree_level_enable_hiz(struct brw_context *brw,
> }
>
>
> +/**
> + * Helper for intel_miptree_alloc_hiz() that determines the required hiz
> + * buffer dimensions and allocates a bo for the hiz buffer.
> + */
> +static struct intel_miptree_aux_buffer *
> +intel_gen7_hiz_buf_create(struct brw_context *brw,
> + struct intel_mipmap_tree *mt)
> +{
> + unsigned z_width = mt->logical_width0;
> + unsigned z_height = mt->logical_height0;
> + const unsigned z_depth = mt->logical_depth0;
> + unsigned hz_width, hz_height;
> + struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
> +
> + if (!buf)
> + return NULL;
> +
> + /* Gen7 PRM Volume 2, Part 1, 11.5.3 "Hierarchical Depth Buffer" documents
> +* adjustments required for Z_Height and Z_Width based on multisampling.
> +*/
> + switch(mt->num_samples) {
I'm new here, but you need a space, I think.
> + case 0:
> + case 1:
> + break;
> + case 2:
> + case 4:
> + z_width *= 2;
> + z_height *= 2;
> + break;
> + case 8:
> + z_width *= 4;
> + z_height *= 2;
> + break;
> + default:
> + assert(!"Unsupported sample count!");
> + }
> +
> + const unsigned vertical_align = 8; /* 'j' in the docs */
j seems to vary based on surface type (if I am reading this correctly).
8 seems safe in all cases.
> + const unsigned H0 = z_height;
> + const unsigned h0 = ALIGN(H0, vertical_align);
> + const unsigned h1 = ALIGN(minify(H0, 1), vertical_align);
I'm having trouble verifying this. On gen8+ it seems this shouldn't be
aligned, ie. h1 = mt->logical_height0. Can you help me find why you are
doing this operation.
> + const unsigned Z0 = z_depth;
> +
> + /* HZ_Width (bytes) = ceiling(Z_Width / 16) * 16 */
> + hz_width = ALIGN(z_width, 16);
> +
> + if (mt->target == GL_TEXTURE_3D) {
> + unsigned H_i = H0;
> + unsigned Z_i = Z0;
> + hz_height = 0;
> + for (int level = mt->first_level; level <= mt->last_level; ++level) {
> + unsigned h_i = ALIGN(H_i, vertical_align);
> + /* sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */
You could optimize this I think, if z_depth is 0 we can ignore this
whole operation. Not sure if that ever happens or not.
> + hz_height += h_i * Z_i;
> + H_i = minify(H_i, 1);
> + Z_i = minify(Z_i, 1);
I want to discuss this with you in person.
> + }
> + /* HZ_Height =
> + *(1/2) * sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i)))
> + */
> + hz_height = CEILING(hz_height, 2);
> + } else {
> + const unsigned hz_qpitch = h0 + h1 + (12 * vertical_align);
> + if (mt->target == GL_TEXTURE_CUBE_MAP_ARRAY ||
> + mt->target == GL_TEXTURE_CUBE_MAP) {
> + /* HZ_Height (rows) = Ceiling ( ( Q_pitch * Z_depth * 6/2) /8 ) * 8
> */
> + hz_height = CEILING(hz_qpitch * Z0 * 6, 2 * 8) * 8;
> + } else {
> + /* HZ_Height (rows) = Ceiling ( ( Q_pitch * Z_depth/2) /8 ) * 8 */
> + hz_height = CEILING(hz_qpitch * Z0, 2 * 8) * 8;
> + }
> + }
> +
> + unsigned long pitch;
> + uint32_t tiling = I915_TILING_Y;
> + buf->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz",
> + hz_width, hz_height, 1,
> + &tiling, &pitch,
> + BO_ALLOC_FOR_RENDER);
I think you need to check tiling here (didn't check pitch).
> + buf->pitch = pitch;
> +
> + return buf;
> +}
> +
> +
> static struct intel_miptree_aux_buffer *
> intel_hiz_miptree_buf_create(struct brw_context *brw,
> struct intel_mipmap_tree *mt)
> @@ -1412,7 +1500,12 @@ intel_miptr
Re: [Mesa-dev] Bogus bounds checking in api_validate.c?
On Mon, 2014-11-10 at 06:09 -0800, Ian Romanick wrote:
> On 11/06/2014 11:40 PM, Kenneth Graunke wrote:
> > On Thursday, November 06, 2014 08:09:18 PM Ian Romanick wrote:
> >> While working on some other things, I came across some bounds checking
> >> code in _mesa_validate_DrawElements (and related functions) in
> >> api_validate.c.
> >>
> >> /* use indices in the buffer object */
> >> /* make sure count doesn't go outside buffer bounds */
> >> if (index_bytes(type, count) > ctx->Array.VAO->IndexBufferObj->Size)
> >> {
> >> _mesa_warning(ctx, "glDrawElements index out of buffer bounds");
> >> return GL_FALSE;
> >> }
> >>
> >> index_bytes calculates how many bytes of data "count" indices will
> >> occupy based on the type. The problem is that this doesn't consider
> >> the base pointer. As far as I can tell, if I had a 64 byte buffer
> >> object for my index data, and I did
> >>
> >> glDrawElements(GL_POINTS, 16, GL_UNSIGNED_INT, 60);
> >>
> >> _mesa_validate_DrawElements would say, "Ok!"
> >>
> >> Am I missing something, or is this just broken?
> >
> > It sure seems broken to me - but, thankfully, in a conservative fashion.
> > (It
> > will say some invalid things are OK, but won't say legal things are
> > invalid.)
> >
> > Software drivers may be relying on this working to avoid a crash.
> >
> > I checked the Ivybridge documentation, and found:
> >
> > "Software is responsible for ensuring that accesses outside the IB do not
> > occur. This is possible as software can compute the range of IB values
> > referenced by a 3DPRIMITIVE command (knowing the StartVertexLocation,
> > InstanceCount, and VerticesPerInstance values) and can then compare this
> > range to the IB extent."
> >
> > which makes it sound like an accurate computation is necessary. But, right
> > below that, it says:
> >
> > "this field contains the address of the last valid byte in the index buffer.
> > Any index buffer reads past this address returns an index value of 0 (as if
> > the index buffer was zero-extended)."
> >
> > So the earlier statement is false; i965 will draw the in-bounds elements
> > correctly, and then repeat element 0 over and over for any out-of-bounds
> > data,
> > resulting in one strange primitive and a lot of degenerate ones.
> >
> > It's proabbly worth fixing, but I doubt it's critical either.
>
> Hmm... I came across this while looking at cachegrind traces of GL
> applications. Time spent in _mesa_validate_Draw* was non-trivial.
> Since at least some hardware doesn't need this check, I think I want to
> move it down into drivers that actually need the check... which is kind
> of a bummer since I came up with a clever optimization for index_bytes.
I'm not sure what applications you looked at in cachegrind but OpenArena
and Nexuiz both spend a lot of time in here. On my Ivybridge:
OpenArena 5.4% out of a total 27.94% in i965_dri.so
Nexuiz 3.28% out of a total of 29.4% in i965_dri.so
For those not up to speed are you able to give a one line explanation of
why some hardware can get away without this check?
>
> > A more interesting thing to fix, I think, would be enforcing alignment
> > restrictions (i.e. your offset has to be a multiple of the IB element size).
>
> That would probably be useful in debug builds, but I'm pretty sure the
> GL spec says the behavior is undefined specifically to avoid the check
> in the hot path.
>
> > --Ken
>
>
> ___
> 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: Use the predicate enable bit for conditional rendering without stalling
Neil Roberts writes: > It looks like the PRM for Haswell says that MI_LOAD_REGISTER_MEM is > converted to no-op for non-privileged buffers. However I can't find any > mention of this for IvyBridge. Does that mean it's allowed on IvyBridge > but it won't work on Haswell? I haven't tested it on Haswell yet. Ok, I just tested it on Haswell and sure enough it doesn't work. It does however fail gracefully because can_do_pipelined_register_writes fails so it doesn't even try to use the predicate registers. I think in that case it wouldn't cause any problems to land the patches as they are. If we later find a way to make it work on Haswell we could bump the command parser version again as you say. We could also use that when deciding whether to enable indirect rendering to avoid having to do a trial load. - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCHv2] auxiliary/vl: rework the build of the VL code
Rather than shoving all the VL code for non-VL targets, increasing
their size, just split it out and use it when needed. This gives us
the side effect of building vl_winsys_dri.c once, dropping a few
automake warnings, and reducing the size of the dri modules as below
textdata bss dec hex filename
5850573 187549 1977928 8016050 7a50b2 before/nouveau_dri.so
5508486 187100 391240 6086826 5ce0aa after/nouveau_dri.so
The above data is for a nouveau + swrast + kms_swrast 'megadriver'.
v2: Do not include the vl sources in the auxiliary library.
Cc: Christian König
Signed-off-by: Emil Velikov
---
src/gallium/auxiliary/Android.mk| 2 +-
src/gallium/auxiliary/Makefile.am | 23 ++-
src/gallium/auxiliary/Makefile.sources | 6 ++
src/gallium/auxiliary/SConscript| 2 +-
src/gallium/targets/dri/Makefile.am | 1 +
src/gallium/targets/egl-static/Makefile.am | 1 +
src/gallium/targets/gbm/Makefile.am | 1 +
src/gallium/targets/omx/Makefile.am | 11 +++
src/gallium/targets/pipe-loader/Makefile.am | 14 --
src/gallium/targets/va/Makefile.am | 11 +++
src/gallium/targets/vdpau/Makefile.am | 11 +++
src/gallium/targets/xa/Makefile.am | 1 +
src/gallium/targets/xvmc/Makefile.am| 11 +++
13 files changed, 54 insertions(+), 41 deletions(-)
diff --git a/src/gallium/auxiliary/Android.mk b/src/gallium/auxiliary/Android.mk
index 2e7d7a8..0bc1831 100644
--- a/src/gallium/auxiliary/Android.mk
+++ b/src/gallium/auxiliary/Android.mk
@@ -30,7 +30,7 @@ include $(CLEAR_VARS)
LOCAL_SRC_FILES := \
$(C_SOURCES) \
- $(VL_SOURCES)
+ $(VL_STUB_SOURCES)
LOCAL_C_INCLUDES := \
$(GALLIUM_TOP)/auxiliary/util \
diff --git a/src/gallium/auxiliary/Makefile.am
b/src/gallium/auxiliary/Makefile.am
index 1e18e6e..7d23cd3 100644
--- a/src/gallium/auxiliary/Makefile.am
+++ b/src/gallium/auxiliary/Makefile.am
@@ -18,7 +18,6 @@ AM_CXXFLAGS = $(VISIBILITY_CXXFLAGS)
libgallium_la_SOURCES = \
$(C_SOURCES) \
- $(VL_SOURCES) \
$(GENERATED_SOURCES)
if HAVE_MESA_LLVM
@@ -52,3 +51,25 @@ util/u_format_table.c: $(srcdir)/util/u_format_table.py
$(srcdir)/util/u_format_
noinst_LTLIBRARIES += libgalliumvl_stub.la
libgalliumvl_stub_la_SOURCES = \
$(VL_STUB_SOURCES)
+
+if NEED_GALLIUM_VL
+
+noinst_LTLIBRARIES += libgalliumvl.la
+
+libgalliumvl_la_CFLAGS = \
+ $(AM_CFLAGS) \
+ $(VL_CFLAGS) \
+ $(LIBDRM_CFLAGS) \
+ $(GALLIUM_PIPE_LOADER_DEFINES) \
+ -DPIPE_SEARCH_DIR=\"$(libdir)/gallium-pipe\"
+
+if HAVE_GALLIUM_STATIC_TARGETS
+libgalliumvl_la_CFLAGS += \
+ -DGALLIUM_STATIC_TARGETS=1
+
+endif # HAVE_GALLIUM_STATIC_TARGETS
+
+libgalliumvl_la_SOURCES = \
+ $(VL_SOURCES)
+
+endif
diff --git a/src/gallium/auxiliary/Makefile.sources
b/src/gallium/auxiliary/Makefile.sources
index 66edb4d..f3b95b9 100644
--- a/src/gallium/auxiliary/Makefile.sources
+++ b/src/gallium/auxiliary/Makefile.sources
@@ -175,13 +175,11 @@ VL_SOURCES := \
vl/vl_video_buffer.c \
vl/vl_video_buffer.h \
vl/vl_vlc.h \
+ vl/vl_winsys.h \
+ vl/vl_winsys_dri.c \
vl/vl_zscan.c \
vl/vl_zscan.h
-# XXX: Add those to VL_SOURCES once we've split it out of libgallium
-# vl/vl_winsys_dri.c \
-# vl/vl_winsys.h \
-
VL_STUB_SOURCES := \
vl/vl_stubs.c
diff --git a/src/gallium/auxiliary/SConscript b/src/gallium/auxiliary/SConscript
index 81c4f4c..4984434 100644
--- a/src/gallium/auxiliary/SConscript
+++ b/src/gallium/auxiliary/SConscript
@@ -36,7 +36,7 @@ env.Depends('util/u_format_table.c', [
source = env.ParseSourceList('Makefile.sources', [
'C_SOURCES',
-'VL_SOURCES',
+'VL_STUB_SOURCES',
'GENERATED_SOURCES'
])
diff --git a/src/gallium/targets/dri/Makefile.am
b/src/gallium/targets/dri/Makefile.am
index 1094ffd..898ab46 100644
--- a/src/gallium/targets/dri/Makefile.am
+++ b/src/gallium/targets/dri/Makefile.am
@@ -43,6 +43,7 @@ gallium_dri_la_LIBADD = \
$(top_builddir)/src/mesa/drivers/dri/common/libdricommon.la \
$(top_builddir)/src/mesa/drivers/dri/common/libmegadriver_stub.la \
$(top_builddir)/src/gallium/state_trackers/dri/libdri.la \
+ $(top_builddir)/src/gallium/auxiliary/libgalliumvl_stub.la \
$(top_builddir)/src/gallium/auxiliary/libgallium.la \
$(top_builddir)/src/gallium/drivers/galahad/libgalahad.la \
$(top_builddir)/src/gallium/drivers/noop/libnoop.la \
diff --git a/src/gallium/targets/egl-static/Makefile.am
b/src/gallium/targets/egl-static/Makefile.am
index 3f0e650..b188f82 100644
--- a/src/gallium/targets/egl-static/Makefile.am
+++ b/src/gallium/targets/egl-static/Makefile.am
@@ -63,6 +63,7 @@ egl_gallium_la_SOURCES = \
egl_gallium_la_LIBADD = \
$(top_builddir)/src/loader/libloader.la \
+ $(top_builddir)/src/gallium/auxiliary/libgalli
Re: [Mesa-dev] [PATCH v2 03/14] i965/gen8: Don't rely directly on the hiz miptree structure
On Mon, Jul 21, 2014 at 11:00:52PM -0700, Jordan Justen wrote:
> We are still allocating a miptree for hiz, but we only use fields from
> intel_miptree_aux_buffer. This will allow us to switch over to not
> allocating a miptree.
>
> Signed-off-by: Jordan Justen
> Reviewed-by: Topi Pohjolainen
> ---
> src/mesa/drivers/dri/i965/gen8_depth_state.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> index b6f373d..8774595 100644
> --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> @@ -89,10 +89,10 @@ emit_depth_packets(struct brw_context *brw,
> } else {
>BEGIN_BATCH(5);
>OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (5 - 2));
> - OUT_BATCH((depth_mt->hiz_buf->mt->pitch - 1) | BDW_MOCS_WB << 25);
> - OUT_RELOC64(depth_mt->hiz_buf->mt->bo,
> + OUT_BATCH((depth_mt->hiz_buf->pitch - 1) | BDW_MOCS_WB << 25);
> + OUT_RELOC64(depth_mt->hiz_buf->bo,
>I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
> - OUT_BATCH(depth_mt->hiz_buf->mt->qpitch >> 2);
> + OUT_BATCH(depth_mt->hiz_buf->qpitch >> 2);
>ADVANCE_BATCH();
> }
>
> --
I would have just combined this with patch 2.
Reviewed-by: Ben Widawsky
--
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] gk20a: use NOUVEAU_BO_GART as VRAM domain
On Wed, Nov 5, 2014 at 5:23 AM, Alexandre Courbot wrote:
> On 10/30/2014 12:29 AM, Ilia Mirkin wrote:
>>
>> On Mon, Oct 27, 2014 at 6:34 AM, Alexandre Courbot
>> wrote:
>>>
>>> GK20A does not have dedicated VRAM, therefore allocating in VRAM can be
>>> sub-optimal and sometimes even harmful. Set its VRAM domain to
>>> NOUVEAU_BO_GART so all objects are allocated in system memory.
>>>
>>> Signed-off-by: Alexandre Courbot
>>> ---
>>> src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 10 ++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>>> b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>>> index ac5823e4a8d5..ad143cd9a140 100644
>>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>>> @@ -620,6 +620,16 @@ nvc0_screen_create(struct nouveau_device *dev)
>>> return NULL;
>>> pscreen = &screen->base.base;
>>>
>>> + /* Recognize chipsets with no VRAM */
>>> + switch (dev->chipset) {
>>> + /* GK20A */
>>> + case 0xea:
>>> + screen->base.vram_domain = NOUVEAU_BO_GART;
>>
>>
>> I think you also want to set vidmem_bindings = 0... although
>> potentially after the |= that's done below. Although I guess that
>> constbuf + command args buf need to be |='d into the sysmem_bindings
>> for this to work out well.
>
>
> Thanks for pointing this out ; I didn't know about vidmem_bindings to be
> honest. Will update and resend.
>
> Apart from this detail, are you ok with the changes brought by these
> patches?
I'm not against them... I do wonder a bit if there isn't some better
way of doing this, but in the absence of such a way, this seems fine.
-ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 02/14] i965/gen7: Don't rely directly on the hiz miptree structure
On Mon, Jul 21, 2014 at 11:00:51PM -0700, Jordan Justen wrote:
> We are still allocating a miptree for hiz, but we only use fields from
> intel_miptree_aux_buffer. This will allow us to switch over to not
> allocating a miptree.
>
> Signed-off-by: Jordan Justen
> Reviewed-by: Topi Pohjolainen
> ---
> src/mesa/drivers/dri/i965/gen7_blorp.cpp| 6 +++---
> src/mesa/drivers/dri/i965/gen7_misc_state.c | 7 ---
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> index d0d623d..ebfe6d1 100644
> --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> @@ -752,13 +752,13 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context
> *brw,
>
> /* 3DSTATE_HIER_DEPTH_BUFFER */
> {
> - struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
> + struct intel_miptree_aux_buffer *hiz_buf = params->depth.mt->hiz_buf;
>
>BEGIN_BATCH(3);
>OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
>OUT_BATCH((mocs << 25) |
> -(hiz_mt->pitch - 1));
> - OUT_RELOC(hiz_mt->bo,
> +(hiz_buf->pitch - 1));
> + OUT_RELOC(hiz_buf->bo,
> I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
> 0);
>ADVANCE_BATCH();
> diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> index 6c6a79b..8f8c33e 100644
> --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> @@ -145,12 +145,13 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
>OUT_BATCH(0);
>ADVANCE_BATCH();
> } else {
> - struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
> + struct intel_miptree_aux_buffer *hiz_buf = depth_mt->hiz_buf;
> +
>BEGIN_BATCH(3);
>OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
>OUT_BATCH((mocs << 25) |
> -(hiz_mt->pitch - 1));
> - OUT_RELOC(hiz_mt->bo,
> +(hiz_buf->pitch - 1));
> + OUT_RELOC(hiz_buf->bo,
> I915_GEM_DOMAIN_RENDER,
> I915_GEM_DOMAIN_RENDER,
> 0);
> --
Just a simple grep turns up a few places that weren't modified. I am
not sure if you intentionally left out the <= gen6 state, but just for
the pedant:
brw_emit_depth_stencil_hiz()
gen6_blorp_emit_depth_stencil_config()
gen6_emit_depth_stencil_hiz()
If it was intentional, maybe adjust the commit message?
I see nothing that would break as a result here though.
Reviewed-by: Ben Widawsky
--
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 01/14] i965/hiz: Start to separate miptree out from hiz buffers
On Mon, Jul 21, 2014 at 11:00:50PM -0700, Jordan Justen wrote:
> Today we allocate a miptree's for the hiz buffer. We needed this in
> the past because we would point the hardware at offsets of the hiz
> buffer. Since the hiz format is not documented, this is not a good
> idea.
>
> Since moving to support layered rendering on Gen7+, we no longer point
> at an offset into the buffer on Gen7+.
>
> Therefore, to support hiz on Gen7+, we don't need a full miptree
> structure allocated.
>
> This patch starts to create a new auxiliary buffer structure
> (intel_miptree_aux_buffer) that can be a more simplistic miptree
> side-band buffer associated with a miptree. (For example, to serve the
> needs of the hiz buffer.)
>
> Signed-off-by: Jordan Justen
> Reviewed-by: Topi Pohjolainen
> ---
> src/mesa/drivers/dri/i965/brw_misc_state.c| 4 +-
> src/mesa/drivers/dri/i965/gen6_blorp.cpp | 2 +-
> src/mesa/drivers/dri/i965/gen7_blorp.cpp | 2 +-
> src/mesa/drivers/dri/i965/gen7_misc_state.c | 2 +-
> src/mesa/drivers/dri/i965/gen8_depth_state.c | 6 +--
> src/mesa/drivers/dri/i965/intel_fbo.c | 4 +-
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 55
> +++
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 29 --
> 8 files changed, 76 insertions(+), 28 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
> b/src/mesa/drivers/dri/i965/brw_misc_state.c
> index 76e22bd..af900cd 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -182,7 +182,7 @@ brw_get_depthstencil_tile_masks(struct intel_mipmap_tree
> *depth_mt,
>
>if (intel_miptree_level_has_hiz(depth_mt, depth_level)) {
> uint32_t hiz_tile_mask_x, hiz_tile_mask_y;
> - intel_miptree_get_tile_masks(depth_mt->hiz_mt,
> + intel_miptree_get_tile_masks(depth_mt->hiz_buf->mt,
>&hiz_tile_mask_x, &hiz_tile_mask_y,
>false);
>
> @@ -643,7 +643,7 @@ brw_emit_depth_stencil_hiz(struct brw_context *brw,
>
>/* Emit hiz buffer. */
>if (hiz) {
> - struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt;
> + struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
>BEGIN_BATCH(3);
>OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
>OUT_BATCH(hiz_mt->pitch - 1);
> diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> index eb865b9..282c7b2 100644
> --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> @@ -855,7 +855,7 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context
> *brw,
>
> /* 3DSTATE_HIER_DEPTH_BUFFER */
> {
> - struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt;
> + struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
>uint32_t hiz_offset =
> intel_miptree_get_aligned_offset(hiz_mt,
>draw_x & ~tile_mask_x,
> diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> index 0ad570b..d0d623d 100644
> --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> @@ -752,7 +752,7 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context
> *brw,
>
> /* 3DSTATE_HIER_DEPTH_BUFFER */
> {
> - struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt;
> + struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
>
>BEGIN_BATCH(3);
>OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
> diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> index 22911bf..6c6a79b 100644
> --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> @@ -145,7 +145,7 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
>OUT_BATCH(0);
>ADVANCE_BATCH();
> } else {
> - struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt;
> + struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
>BEGIN_BATCH(3);
>OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
>OUT_BATCH((mocs << 25) |
> diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> index 7c3bfe0..b6f373d 100644
> --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> @@ -89,10 +89,10 @@ emit_depth_packets(struct brw_context *brw,
> } else {
>BEGIN_BATCH(5);
>OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (5 - 2));
> - OUT_BATCH((depth_mt->hiz_mt->pitch - 1) | BDW_MOCS_WB << 25);
> - OUT_RELOC64(depth_mt->hiz_mt->bo,
> + OUT_BATCH((depth_mt->hiz_buf->mt->pitch - 1) | BDW_MOCS_WB << 25);
> + OU
Re: [Mesa-dev] [PATCH 2/2] clover: clCompileProgram CL_INVALID_COMPILER_OPTIONS
EdB writes:
> clCompileProgram should return CL_INVALID_COMPILER_OPTIONS
> instead of CL_INVALID_BUILD_OPTIONS
Looks good to me,
Reviewed-by: Francisco Jerez
> ---
> src/gallium/state_trackers/clover/api/program.cpp | 2 ++
> src/gallium/state_trackers/clover/llvm/invocation.cpp | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/clover/api/program.cpp
> b/src/gallium/state_trackers/clover/api/program.cpp
> index 3a6c054..60184ed 100644
> --- a/src/gallium/state_trackers/clover/api/program.cpp
> +++ b/src/gallium/state_trackers/clover/api/program.cpp
> @@ -182,6 +182,8 @@ clBuildProgram(cl_program d_prog, cl_uint num_devs,
> prog.build(devs, opts);
> return CL_SUCCESS;
> } catch (error &e) {
> + if (e.get() == CL_INVALID_COMPILER_OPTIONS)
> + return CL_INVALID_BUILD_OPTIONS;
> if (e.get() == CL_COMPILE_PROGRAM_FAILURE)
>return CL_BUILD_PROGRAM_FAILURE;
> return e.get();
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index d29f5a6..30547d0 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -177,7 +177,7 @@ namespace {
> opts_carray.data() +
> opts_carray.size(),
> Diags);
>if (!Success) {
> - throw error(CL_INVALID_BUILD_OPTIONS);
> + throw error(CL_INVALID_COMPILER_OPTIONS);
>}
>c.getFrontendOpts().ProgramAction = clang::frontend::EmitLLVMOnly;
>c.getHeaderSearchOpts().UseBuiltinIncludes = true;
> --
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
pgps1_KwDC_Lv.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] clover: fix tab/space and alignement
EdB writes:
> ---
> src/gallium/state_trackers/clover/llvm/invocation.cpp | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index 3a4fcf0..d29f5a6 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -214,7 +214,7 @@ namespace {
>
> #if HAVE_LLVM >= 0x0306
>c.getPreprocessorOpts().addRemappedFile(name,
> -
> llvm::MemoryBuffer::getMemBuffer(source).release());
> +
> llvm::MemoryBuffer::getMemBuffer(source).release());
This doesn't to fix the inconsistency that the line goes over the 80
column limit, if that's what you meant to fix, and it makes the
alignment with the open parenthesis inconsistent.
> #else
>c.getPreprocessorOpts().addRemappedFile(name,
>
> llvm::MemoryBuffer::getMemBuffer(source));
> @@ -675,7 +675,7 @@ static const struct debug_named_value debug_options[] = {
> {"llvm", DBG_LLVM, "Dump the generated LLVM IR for all kernels."},
> {"asm", DBG_ASM, "Dump kernel assembly code for targets specifying "
> "PIPE_SHADER_IR_NATIVE"},
> - DEBUG_NAMED_VALUE_END // must be last
> + DEBUG_NAMED_VALUE_END // must be last
> };
>
> module
> @@ -737,7 +737,7 @@ clover::compile_program_llvm(const compat::string &source,
> break;
>case PIPE_SHADER_IR_NATIVE: {
> std::vector code = compile_native(mod, triple, processor,
> - debug_flags & DBG_ASM, r_log);
> + debug_flags & DBG_ASM,
> r_log);
> m = build_module_native(code, mod, kernels, address_spaces, r_log);
> break;
>}
> --
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
pgpkYNotTtMsk.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Fix segfault in WebGL Conformance on Ivybridge
Fixes regression of WebGL Conformance test texture-size-limit [1] on
Ivybridge Mobile GT2 0x0166 with Google Chrome R38.
Regression introduced by
commit 6c044231535b93c5d16404528946cad618d96bd9
Author: Kenneth Graunke
Date: Sun Feb 2 02:58:42 2014 -0800
i965: Bump GL_MAX_CUBE_MAP_TEXTURE_SIZE to 8192.
The test regressed because the pointer offset arithmetic in
intel_miptree_map_gtt() overflows for large textures. The pointer
arithmetic is not 64-bit safe.
This patch fixes the bugzilla ticket below on Ivybridge. This patch
doesn't close the ticket, though, because the bug report is against
Sandybridge, and QA cannot confirm the fix on that hardware.
[1]
https://github.com/KhronosGroup/WebGL/blob/52f0dc240f04dce31b1b8e2b8107fe2b8332dc90/sdk/tests/conformance/textures/texture-size-limit.html
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78770
Fixes: Intel CHRMOS-1377
Reported-by: Lu Hua
Signed-off-by: Chad Versace
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 8fda25d..24e217c 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1769,7 +1769,16 @@ intel_miptree_map_gtt(struct brw_context *brw,
y += image_y;
map->stride = mt->pitch;
- map->ptr = base + y * map->stride + x * mt->cpp;
+
+ /* The variables in below pointer arithmetic are 32-bit. The arithmetic
+ * overflows for large textures. Therefore the cast to intptr_t is
+ * needed.
+ *
+ * TODO(chadv): Fix this everywhere in i965 by fixing the signature of
+ * intel_miptree_get_image_offset() to use intptr_t.
+ */
+ map->ptr = base + (intptr_t) y * (intptr_t) map->stride
+ + (intptr_t) x * (intptr_t) mt->cpp;
}
DBG("%s: %d,%d %dx%d from mt %p (%s) %d,%d = %p/%d\n", __FUNCTION__,
--
2.1.0-rc0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Use the predicate enable bit for conditional rendering without stalling
On Mon, Nov 10, 2014 at 2:57 PM, Neil Roberts wrote: > > The bit I mentioned about OACONTROL was just saying that the method of > detecting whether we can write to OACONTROL specifically doesn't work. > This is because writing to a register that is not in the whitelist > returns EINVAL and Mesa calls exit(1) when drm_intel_gem_bo_context_exec > fails. I used a different method to detect whether we can write to > MI_PREDICATE_SRC0 by manually calling drm_intel_gem_bo_context_exec to > avoid the call to exit(1). I think the code to check whether OACONTROL > is accessible is wrong because the process will have already exited > before it gets a chance to check whether the load worked. In v2 of the > patch I made it check the cmd parser version as you suggested. We should > probably do something similar for OACONTROL, but that is a separate > issue. Hmm, that's awkward. I had been thinking we should aim remove the OACONTROL tricks from the cmd parser, considering that it's not currently possible to program the OA to get useful data this way, but it sounds like we'd actually start making Mesa apps exit(1) if we were to do that :-/ If we add more complete support for the OA via perf then it would be nice if we just had one place responsible for programming OACONTROL so I'd been thinking we could just drop it from the cmd parser white list - assuming older versions of Mesa would handle that gracefully. 'Guess it won't be quite so simple :-) Regards, - Robert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Use the predicate enable bit for conditional rendering without stalling
> On Fri, Nov 07, 2014 at 03:28:01PM +, Neil Roberts wrote: >> Unfortunately these two source registers are not in the whitelist of >> available registers in the kernel driver so this needs a kernel patch >> to work. This patch tries to check whether it is possible to write to >> this register by creating a DRM buffer with a single command to write >> to the register and then trying to exec it. The return value of the >> exec function (and hence the ioctl) is checked. >> >> There is a function with a similar goal just above to check whether >> the OACONTROL register can be used. This works by putting the test >> command in the regular batch buffer and trying to compare whether the >> value was successfully written. I couldn't get this approach to work >> because intel_batchbuffer_flush actually calls exit() if the ioctl >> fails. I am suspicious that this approach doesn't work for OACONTROL >> either. Daniel Vetter writes: > Atm the cmd parser for gen7 validates the batch and rejects it if there's > something in there it doesn't like. But it doesn't grant any additional > privs. Hence > - OACONTROL passes since it's in the whitelist, but since the cmd parser > doesn't grant the needed privs the writes would need. > - your new regs aren't in the whitelist, so the execbuf fails with > -EINVAL. > > We have a cmd parser version which we intend to bump every time we add new > registers, so probably better to check that. And I guess we'd need one to > indicate that the cmd parser actually does something useful. Probably best > done with just another bump. And if we do that we could replace the > current trick mesa uses with just a getparam query - the getparam is fixed > so either returns -EINVAL on old kernels or the cmd parser version. I'm not really sure if I understand your reply correctly. Did you see that I also posted the corresponding kernel patch? I was testing my patch on top of the drm-intel-next branch with IvyBridge. If I don't apply the patch then I get EINVAL and the register load doesn't work. If I do apply it then everything seems to work and the Piglit tests pass. Therefore I'm assuming that the only thing that's stopping the register load from working without the patch is the whitelist in the command parser so adding the registers to the whitelist should be enough to get it working. It looks like the PRM for Haswell says that MI_LOAD_REGISTER_MEM is converted to no-op for non-privileged buffers. However I can't find any mention of this for IvyBridge. Does that mean it's allowed on IvyBridge but it won't work on Haswell? I haven't tested it on Haswell yet. However, as well as checking the command parser version, the patch for Mesa only enables using the predicate registers if it determines it can do a register write from an untrusted buffer. This is done via can_do_pipelined_register_writes which already existed to determine whether we can do indirect renders. So presumably checking for this and checking for version 2 of the command parser would be enough to ensure that we can load into the predicate register. Presumably it would also start working magically on Haswell if we did something to disable the hardware command parser and only relied on the kernel command parser. Is that the plan? The bit I mentioned about OACONTROL was just saying that the method of detecting whether we can write to OACONTROL specifically doesn't work. This is because writing to a register that is not in the whitelist returns EINVAL and Mesa calls exit(1) when drm_intel_gem_bo_context_exec fails. I used a different method to detect whether we can write to MI_PREDICATE_SRC0 by manually calling drm_intel_gem_bo_context_exec to avoid the call to exit(1). I think the code to check whether OACONTROL is accessible is wrong because the process will have already exited before it gets a chance to check whether the load worked. In v2 of the patch I made it check the cmd parser version as you suggested. We should probably do something similar for OACONTROL, but that is a separate issue. Regards, - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Bogus bounds checking in api_validate.c?
On 11/10/2014 06:09 AM, Ian Romanick wrote:
> On 11/06/2014 11:40 PM, Kenneth Graunke wrote:
>> On Thursday, November 06, 2014 08:09:18 PM Ian Romanick wrote:
>>> While working on some other things, I came across some bounds checking
>>> code in _mesa_validate_DrawElements (and related functions) in
>>> api_validate.c.
>>>
>>> /* use indices in the buffer object */
>>> /* make sure count doesn't go outside buffer bounds */
>>> if (index_bytes(type, count) > ctx->Array.VAO->IndexBufferObj->Size) {
>>> _mesa_warning(ctx, "glDrawElements index out of buffer bounds");
>>> return GL_FALSE;
>>> }
>>>
>>> index_bytes calculates how many bytes of data "count" indices will
>>> occupy based on the type. The problem is that this doesn't consider
>>> the base pointer. As far as I can tell, if I had a 64 byte buffer
>>> object for my index data, and I did
>>>
>>> glDrawElements(GL_POINTS, 16, GL_UNSIGNED_INT, 60);
>>>
>>> _mesa_validate_DrawElements would say, "Ok!"
>>>
>>> Am I missing something, or is this just broken?
>>
>> It sure seems broken to me - but, thankfully, in a conservative fashion.
>> (It
>> will say some invalid things are OK, but won't say legal things are invalid.)
>>
>> Software drivers may be relying on this working to avoid a crash.
>>
>> I checked the Ivybridge documentation, and found:
>>
>> "Software is responsible for ensuring that accesses outside the IB do not
>> occur. This is possible as software can compute the range of IB values
>> referenced by a 3DPRIMITIVE command (knowing the StartVertexLocation,
>> InstanceCount, and VerticesPerInstance values) and can then compare this
>> range to the IB extent."
>>
>> which makes it sound like an accurate computation is necessary. But, right
>> below that, it says:
>>
>> "this field contains the address of the last valid byte in the index buffer.
>> Any index buffer reads past this address returns an index value of 0 (as if
>> the index buffer was zero-extended)."
>>
>> So the earlier statement is false; i965 will draw the in-bounds elements
>> correctly, and then repeat element 0 over and over for any out-of-bounds
>> data,
>> resulting in one strange primitive and a lot of degenerate ones.
>>
>> It's proabbly worth fixing, but I doubt it's critical either.
>
> Hmm... I came across this while looking at cachegrind traces of GL
> applications. Time spent in _mesa_validate_Draw* was non-trivial.
> Since at least some hardware doesn't need this check, I think I want to
> move it down into drivers that actually need the check... which is kind
> of a bummer since I came up with a clever optimization for index_bytes.
>
>> A more interesting thing to fix, I think, would be enforcing alignment
>> restrictions (i.e. your offset has to be a multiple of the IB element size).
>
> That would probably be useful in debug builds, but I'm pretty sure the
> GL spec says the behavior is undefined specifically to avoid the check
> in the hot path.
You may be right, actually. Section 6.2 (Creating and Modifying Buffer
Object Data Stores) of the 4.5 spec says:
"Clients must align data elements consistent with the requirements
of the client platform, with an additional base-level requirement
that an offset within a buffer to a datum comprising N basic
machine units be a multiple of N."
Of course, it doesn't say what to do. I don't see anything in this
section or in section 2.3.1 (Errors) that suggests an error to generate.
Just ignore the command? Halt and catch fire?
We have some lovely re-base code in brw_upload_indices that maybe we can
delete.
>> --Ken
>
> ___
> 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] Bogus bounds checking in api_validate.c?
On 11/06/2014 11:40 PM, Kenneth Graunke wrote:
> On Thursday, November 06, 2014 08:09:18 PM Ian Romanick wrote:
>> While working on some other things, I came across some bounds checking
>> code in _mesa_validate_DrawElements (and related functions) in
>> api_validate.c.
>>
>> /* use indices in the buffer object */
>> /* make sure count doesn't go outside buffer bounds */
>> if (index_bytes(type, count) > ctx->Array.VAO->IndexBufferObj->Size) {
>> _mesa_warning(ctx, "glDrawElements index out of buffer bounds");
>> return GL_FALSE;
>> }
>>
>> index_bytes calculates how many bytes of data "count" indices will
>> occupy based on the type. The problem is that this doesn't consider
>> the base pointer. As far as I can tell, if I had a 64 byte buffer
>> object for my index data, and I did
>>
>> glDrawElements(GL_POINTS, 16, GL_UNSIGNED_INT, 60);
>>
>> _mesa_validate_DrawElements would say, "Ok!"
>>
>> Am I missing something, or is this just broken?
>
> It sure seems broken to me - but, thankfully, in a conservative fashion. (It
> will say some invalid things are OK, but won't say legal things are invalid.)
>
> Software drivers may be relying on this working to avoid a crash.
>
> I checked the Ivybridge documentation, and found:
>
> "Software is responsible for ensuring that accesses outside the IB do not
> occur. This is possible as software can compute the range of IB values
> referenced by a 3DPRIMITIVE command (knowing the StartVertexLocation,
> InstanceCount, and VerticesPerInstance values) and can then compare this
> range to the IB extent."
>
> which makes it sound like an accurate computation is necessary. But, right
> below that, it says:
>
> "this field contains the address of the last valid byte in the index buffer.
> Any index buffer reads past this address returns an index value of 0 (as if
> the index buffer was zero-extended)."
>
> So the earlier statement is false; i965 will draw the in-bounds elements
> correctly, and then repeat element 0 over and over for any out-of-bounds
> data,
> resulting in one strange primitive and a lot of degenerate ones.
>
> It's proabbly worth fixing, but I doubt it's critical either.
Hmm... I came across this while looking at cachegrind traces of GL
applications. Time spent in _mesa_validate_Draw* was non-trivial.
Since at least some hardware doesn't need this check, I think I want to
move it down into drivers that actually need the check... which is kind
of a bummer since I came up with a clever optimization for index_bytes.
> A more interesting thing to fix, I think, would be enforcing alignment
> restrictions (i.e. your offset has to be a multiple of the IB element size).
That would probably be useful in debug builds, but I'm pretty sure the
GL spec says the behavior is undefined specifically to avoid the check
in the hot path.
> --Ken
signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: dri2: Use present extension. (Was: Re: [RFC] egl: Add DRI3 support to the EGL backend.)
Hi, On pe, 2014-11-07 at 17:40 -0800, Eric Anholt wrote: > Ian Romanick writes: > > > On 11/06/2014 06:16 PM, Michel Dänzer wrote: > >> On 06.11.2014 19:18, Joonas Lahtinen wrote: > >>> On to, 2014-11-06 at 18:12 +0900, Michel Dänzer wrote: > On 05.11.2014 20:14, Joonas Lahtinen wrote: > > > > Modified not refer to DRI3, just uses the present extension to get rid > > of the excess buffer invalidations. > > AFAICT there's no fallback from your changes to the current behaviour if > the X server doesn't support the Present extension. There probably needs > to be such a fallback. > >>> > >>> It gets rid of such nasty hack (the intel_viewport one), that I thought > >>> there is no point making fallback. Because without this, the egl dri2 > >>> backend is fundamentally broken anyway. > >> > >> Well, AFAICT your code uses Present extension functionality > >> unconditionally, without checking that the X server supports Present. I > >> can't see how that could possibly work on an X server which doesn't > >> support Present, but I think it would be better to keep it working at > >> least as badly as it does now in that case. :) > > > > I was going to say pretty much the same thing. Aren't there (non-Intel) > > drivers that don't do Present? If I'm not mistaken, some parts of DRI3 > > (not sure about Present) are even disabled in the Intel driver when SNA > > is in use... or at least that was the case at one point. > > They actually get a fallback implementation if there's no driver > support, which would be sufficient for this code. > > However, Present is too new for Mesa to be unconditionally relying on in > my opinion. Based on above discussion, I would bring back the dynamic detection like in the original patch. But for present extension instead of DRI3. Technically it would be very much the same, different naming conventions. And also, re-use the USE_INVALIDATE extension instead of adding DRI3 extension. Would that be an acceptable solution? Regards, Joonas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 54080] glXQueryDrawable fails with GLXBadDrawable for a Window in direct context
https://bugs.freedesktop.org/show_bug.cgi?id=54080 Kevin Rogovin changed: What|Removed |Added Priority|medium |high Severity|normal |major --- Comment #9 from Kevin Rogovin --- Makes tools hard. -- 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] [PATCH] gallium: remove unused pipe_viewport_state::translate[3] and scale[3]
Yes, you're right. I'll fix that. Marek On Mon, Nov 10, 2014 at 12:53 AM, Ilia Mirkin wrote: > On Sun, Nov 9, 2014 at 6:39 PM, Marek Olšák wrote: >> >> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c >> b/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c >> index f227559..6df0c47 100644 >> --- a/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c >> +++ b/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c >> @@ -254,11 +254,11 @@ nv30_validate_viewport(struct nv30_context *nv30) >> PUSH_DATAf(push, vp->translate[0]); >> PUSH_DATAf(push, vp->translate[1]); >> PUSH_DATAf(push, vp->translate[2]); >> - PUSH_DATAf(push, vp->translate[3]); >> + PUSH_DATAf(push, 1.0f); > > > This should probably be 0.0f... or am I misunderstanding what this does? > >> >> PUSH_DATAf(push, vp->scale[0]); >> PUSH_DATAf(push, vp->scale[1]); >> PUSH_DATAf(push, vp->scale[2]); >> - PUSH_DATAf(push, vp->scale[3]); >> + PUSH_DATAf(push, 1.0f); >> BEGIN_NV04(push, NV30_3D(DEPTH_RANGE_NEAR), 2); >> PUSH_DATAf(push, vp->translate[2] - fabsf(vp->scale[2])); >> PUSH_DATAf(push, vp->translate[2] + fabsf(vp->scale[2])); > > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
