Re: [Mesa-dev] [PATCH] st/dri: Remove useless flush front.
- Original Message - > On Tue, Jan 17, 2012 at 15:47, Stéphane Marchesin > wrote: > > On Wed, Jan 11, 2012 at 10:13, Jose Fonseca > > wrote: > >> - Original Message - > >>> Where else would the flush go? > >> > >> Maybe one of the callers already invoked glFlush, or something > >> like that. > >> > >>> I'll look into fixing it differently > >>> but if the flush is anywhere in the makecurrent path, the same > >>> issue > >>> will happen. Again, none of the classic dri drivers do it. > >> > >> If you wanna a short term fix, it might be OK with just commenting > >> out the flush call, but please leave a note there saying that a > >> flush is needed for strict conformance. > >> > > > > Ok I'll push this for now as it fixes a bunch of Chrome bugs. I > > guess > > I'm confused why no other drivers do it, but maybe they're just all > > non-conformant > > > > Ah I understand how things work a little better now. I see two cases > where the removal of this flush could be an issue: > - The unbind-after-destroy case. In that case we're good since the > flush has happened in dri_destroy_context already. > - Makecurrent path. Again in that case dri_make_current flushes the > old context. > > Therefore I think the flush in dri_unbind_context is not needed. > Please let me know if you see a flaw in this analysis. It looks you got all covered. And it would explain why other drivers don't need it. So your patch looks good. Before commiting, please put a comment describing that flush already happens elsewhere, so that this issue doesn't pop up again. Thanks Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] r600g: fix streamout on evergreen
Enable it in the evergreen_context_draw if needed. Same as already done in the r600_context_draw for r6xx/r7xx. Signed-off-by: Vadim Girlin --- src/gallium/drivers/r600/evergreen_hw_context.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/gallium/drivers/r600/evergreen_hw_context.c b/src/gallium/drivers/r600/evergreen_hw_context.c index e75eaf2..9401d82 100644 --- a/src/gallium/drivers/r600/evergreen_hw_context.c +++ b/src/gallium/drivers/r600/evergreen_hw_context.c @@ -1168,6 +1168,12 @@ void evergreen_context_draw(struct r600_context *ctx, const struct r600_draw *dr r600_context_block_resource_emit_dirty(ctx, dirty_block); } + /* Enable stream out if needed. */ + if (ctx->streamout_start) { + r600_context_streamout_begin(ctx); + ctx->streamout_start = FALSE; + } + /* draw packet */ pm4 = &ctx->pm4[ctx->pm4_cdwords]; pm4[0] = PKT3(PKT3_INDEX_TYPE, 0, ctx->predicate_drawing); -- 1.7.7.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO
When rendering to FBO, rendering is inverted. At the same time, we would also make sure the point sprite origin is inverted. Or, we will get an inverted result correspoinding to rendering to the default winsys FBO. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613 Signed-off-by: Yuanhan Liu --- src/mesa/drivers/dri/i965/brw_defines.h |1 + src/mesa/drivers/dri/i965/gen6_sf_state.c | 19 +-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h index 4d90a99..029be87 100644 --- a/src/mesa/drivers/dri/i965/brw_defines.h +++ b/src/mesa/drivers/dri/i965/brw_defines.h @@ -1128,6 +1128,7 @@ enum brw_message_target { /* DW1 (for gen6) */ # define GEN6_SF_NUM_OUTPUTS_SHIFT 22 # define GEN6_SF_SWIZZLE_ENABLE(1 << 21) +# define GEN6_SF_POINT_SPRITE_UPPERLEFT(0 << 20) # define GEN6_SF_POINT_SPRITE_LOWERLEFT(1 << 20) # define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT 11 # define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT 4 diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c index 548c5a3..d354a2b 100644 --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c @@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw) float point_size; uint16_t attr_overrides[FRAG_ATTRIB_MAX]; bool userclip_active; + int point_sprite_origin; /* _NEW_TRANSFORM */ userclip_active = (ctx->Transform.ClipPlanesEnabled != 0); @@ -258,8 +259,22 @@ upload_sf_state(struct brw_context *brw) /* Clamp to the hardware limits and convert to fixed point */ dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3); - if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT) - dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT; + /* +* When rendering to FBO, rendering is inverted. At the same time, +* we would also make sure the point sprite origin is inverted. +* Or, we will get an inverted result corresponding to rendering +* to the default/window FBO. +*/ + if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT) { + point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT; + if (render_to_fbo) + point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT; + } else { + point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT; + if (render_to_fbo) + point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT; + } + dw1 |= point_sprite_origin; /* _NEW_LIGHT */ if (ctx->Light.ProvokingVertex != GL_FIRST_VERTEX_CONVENTION) { -- 1.7.4.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO
On Wed, Jan 18, 2012 at 06:23:52PM +0800, Yuanhan Liu wrote: > When rendering to FBO, rendering is inverted. At the same time, we would > also make sure the point sprite origin is inverted. Or, we will get an > inverted result correspoinding to rendering to the default winsys FBO. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613 Actually, I'm not sure about this patch, and I would like to treat this patch as a _workaround_. So, any comments are welcome. Thanks, Yuanhan Liu ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 24572] Can't compile git drm/linux-core/drm_memory.c
https://bugs.freedesktop.org/show_bug.cgi?id=24572 Michel Dänzer changed: What|Removed |Added AssignedTo|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop |org |.org Product|Mesa|DRI Version|git |DRI CVS Component|Other |DRM/other -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- 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] Revert "mesa: check depth, stencil formats (not depths) in glBlitFramebuffer"
On 01/17/2012 11:00 PM, Kenneth Graunke wrote: On 01/17/2012 05:27 PM, Ian Romanick wrote: On 01/17/2012 05:12 PM, Chad Versace wrote: This reverts commit 3f1fab06844f696de44d9a56e83ff62e8ea576bd. This loosens the format validation in glBlitFramebuffer. When blitting depth bits, don't require an exact match between the depth formats; only require that the two formats have the same number of depth bits. Ditto for stencil. Fixes Piglit fbo/fbo-blit-d24s8 on Intel drivers with separate stencil enabled. The problem was that, on Intel drivers with separate stencil, the default framebuffer has separate depth and stencil buffers with formats X8_Z24 and S8. The test attempts to blit the depth bits from a S8_Z24 buffer into the default framebuffer. Between S8_Z24 buffers, the EXT_framebuffer_blit spec allows glBlitFramebuffer to blit the depth and stencil bits separately. So I see no reason to prevent blitting the depth bits between X8_Z24 and S8_Z24 or the stencil bits between S8 and S8_Z24. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44665 Note: This is a candidate for the 8.0 branch. CC: Brian Paul Reported-by: Xunx Fang Signed-off-by: Chad Versace --- src/mesa/main/fbobject.c | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index 0524959..9a5e780 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -2709,9 +2709,10 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, if ((readRb == NULL) || (drawRb == NULL)) { mask&= ~GL_STENCIL_BUFFER_BIT; } - else if (readRb->Format != drawRb->Format) { + else if (_mesa_get_format_bits(readRb->Format, GL_STENCIL_BITS) != + _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) { _mesa_error(ctx, GL_INVALID_OPERATION, - "glBlitFramebufferEXT(stencil buffer format mismatch)"); + "glBlitFramebufferEXT(stencil buffer size mismatch)"); return; } } @@ -2731,9 +2732,10 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, if ((readRb == NULL) || (drawRb == NULL)) { mask&= ~GL_DEPTH_BUFFER_BIT; } - else if (readRb->Format != drawRb->Format) { + else if (_mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS) != + _mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS)) { I think the check needs to be slightly stronger than this. This would allow Z32 and Z32_FLOAT which is outside the intention of the spec. Do we want to allow "X-types", like S8Z24->X8Z24 and Yes, I think we want to support that type of case. ARGB->XRGB, Yes, the spec explicitly allows color format conversion: "If the color formats of the read and draw framebuffers do not match, and includes COLOR_BUFFER_BIT, the pixel groups are converted to match the destination format as in CopyPixels, except that no pixel transfer operations apply and clamping behaves as if CLAMP_FRAGMENT_COLOR_ARB is set to FIXED_ONLY_ARB." It's the Z / stencil case that's a bit vague and the reason why I changed the test in the first place: "Calling BlitFramebufferEXT will result in an INVALID_OPERATION error if includes DEPTH_BUFFER_BIT or STENCIL_BUFFER_BIT and the source and destination depth and stencil buffer formats do not match." but not general format mismatches (like Z32<->Z32_FLOAT)? That should probably not be allowed (but it would be interesting to test with an AMD or NVIDIA driver to see what they do). I think you just need to test _mesa_get_format_datatype(src) == _mesa_get_format_datatype(dst) in addition to the depth comparison. Thinking about FBOs...I could see raising an error if the user explicitly made ARGB and XRGB FBOs, but if they chose "RGB" and the driver internally decided to use one or the other, it seems wrong to throw an error. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] dd_function_table::Clear()
While implementing Clear() for a platform, which buffers in GLContext need to be cleared out of the list of: DrawBuffer ReadBuffer WinSysDrawBuffer WinSysReadBuffer Or do they all need to be cleared? A. Demetrious Sharpe ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] dd_function_table::Clear()
On 01/18/2012 09:01 AM, Dee Sharpe wrote: While implementing Clear() for a platform, which buffers in GLContext need to be cleared out of the list of: DrawBuffer ReadBuffer WinSysDrawBuffer WinSysReadBuffer Or do they all need to be cleared? Clearing effects "draw" buffers, not "read" buffers. Unless a user-created FBO is bound, ctx->WinSysDrawBuffer == ctx->DrawBuffer. You need to clear the color buffers attached to ctx->DrawBuffer. The number of buffers to clear is ctx->DrawBuffer->_NumColorDrawBuffers and the indexes/pointers to the renderbuffers is in _ColorDrawBufferIndexes[] and _ColorDrawBuffers[]. I may actually get rid of the later (redundant) array and just write a helper function that returns a pointer to a renderbuffer given a buffer index. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/vs: Take attributes into account when deciding urb_entry_size.
On Tue, 17 Jan 2012 22:10:07 -0800, Kenneth Graunke wrote: > Both the VF and VS share space in the URB. First, the VF stores > attributes (shader inputs) there. The VS then reads the attributes, > executes, and reuses the space to store varyings (shader outputs). > > Thus, we need to calculate the amount of URB space necessary for inputs, > outputs, and pick whichever is greater. > > The old VS backend correctly did this (brw_vs_emit.c:408), but the new > VS backend only considered outputs. > > Fixes vertex scrambling in GLBenchmark PRO on Ivybridge. > > NOTE: This is a candidate for the 8.0 branch. Reviewed-by: Eric Anholt pgpNQRzEa0bDy.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 44039] Mesa 7.12-devel implementation error: unexpected format 0x822e in _mesa_choose_tex_format()
https://bugs.freedesktop.org/show_bug.cgi?id=44039 --- Comment #7 from Brian Paul 2012-01-18 09:47:45 PST --- I've pushed the 16-bit float fallback work-around. Commit 7628696004515074594d4fdac4e422c81c86b32c Nikita, Mathieu: can you re-test with Mesa/master? -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- 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 1/4] glapi: remove non-existent parameter references
On Tue, Jan 17, 2012 at 6:10 PM, Brian Paul wrote: > On 01/16/2012 04:45 PM, nobled wrote: >> >> glGetTexImage, for example, has no width/height/depth parameters. >> >> Also, copy some missing parameter info from the original versions >> of certain functions over to their ARB_robustness counterparts. > > > These look OK to me. There's no real change in the generated .[ch] files, > right? I can't actually tell; whenever I try to run `make` in src/mapi/glapi/gen/, it errors out with "Set XORG_BASE env var" instead. > > -Brian > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: use GL_MAP_INVALIDATE_RANGE_BIT in glTexImage paths
On Tue, 17 Jan 2012 08:19:44 -0700, Brian Paul wrote: > Update the dd.h docs to indicate that GL_MAP_INVALIDATE_RANGE_BIT > can be used with GL_MAP_WRITE_BIT when mapping renderbuffers and > texture images. > > Pass the flag when mapping texture images for glTexImage, glTexSubImage, > etc. It's up to drivers whether to actually make use of the flag. Reviewed-by: Eric Anholt pgppWadGsMLNr.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/7] i965: use AM_CPPFLAGS instead of AM_CFLAGS/CXXFLAGS
On Tue, 17 Jan 2012 15:58:11 -0500, Matt Turner wrote: As far as I can tell, the point of AM_CPPFLAGS existing is if you want to make a target that separately calls the preprocessor, so you a list of includes and a list of non-preprocessor compiler flags. Given that we don't have any preprocessor invocations, we're sure to leak actual cflags into the cppflags, so I don't see why we'd make this change.. pgpYyyaleBhko.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] r600g: fixup AR handling (V2)
From: Dave Airlie So it appears R600s (except rv670) do AR handling different using a different opcode. This patch fixes up r600g to work properly on r600. This fixes ~100 piglit tests here (in GLSL1.30 mode) on rv610. Signed-off-by: Dave Airlie --- src/gallium/drivers/r600/r600_asm.c| 37 ++- src/gallium/drivers/r600/r600_asm.h|7 +- src/gallium/drivers/r600/r600_shader.c |6 - 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/gallium/drivers/r600/r600_asm.c b/src/gallium/drivers/r600/r600_asm.c index 8234744..2a20011 100644 --- a/src/gallium/drivers/r600/r600_asm.c +++ b/src/gallium/drivers/r600/r600_asm.c @@ -94,6 +94,7 @@ static inline unsigned int r600_bytecode_get_num_operands(struct r600_bytecode * case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOV: case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA: case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_FLOOR: + case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT: case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_INT: case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FRACT: case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLOOR: @@ -249,10 +250,11 @@ static struct r600_bytecode_tex *r600_bytecode_tex(void) return tex; } -void r600_bytecode_init(struct r600_bytecode *bc, enum chip_class chip_class) +void r600_bytecode_init(struct r600_bytecode *bc, enum chip_class chip_class, unsigned ar_handling) { LIST_INITHEAD(&bc->cf); bc->chip_class = chip_class; + bc->ar_handling = ar_handling; } static int r600_bytecode_add_cf(struct r600_bytecode *bc) @@ -478,6 +480,7 @@ static int is_alu_trans_unit_inst(struct r600_bytecode *bc, struct r600_bytecode case R700: if (!alu->is_op3) return alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_ASHR_INT || + alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT || alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_INT_TO_FLT || alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLT_TO_UINT || alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLT_TO_INT || @@ -1236,12 +1239,42 @@ static int r600_bytecode_alloc_kcache_lines(struct r600_bytecode *bc, struct r60 return 0; } + +/* load AR register from gpr (bc->ar_reg) with MOVA_INT */ +static int load_ar_r6xx(struct r600_bytecode *bc) +{ + struct r600_bytecode_alu alu; + int r; + + if (bc->ar_loaded) + return 0; + + /* hack to avoid making MOVA the last instruction in the clause */ + if ((bc->cf_last->ndw>>1) >= 110) + bc->force_add_cf = 1; + + memset(&alu, 0, sizeof(alu)); + alu.inst = V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT; + alu.src[0].sel = bc->ar_reg; + alu.last = 1; + r = r600_bytecode_add_alu(bc, &alu); + if (r) + return r; + + bc->cf_last->r6xx_uses_waterfall = 1; + bc->ar_loaded = 1; + return 0; +} + /* load AR register from gpr (bc->ar_reg) with MOVA_INT */ static int load_ar(struct r600_bytecode *bc) { struct r600_bytecode_alu alu; int r; + if (bc->ar_handling) + return load_ar_r6xx(bc); + if (bc->ar_loaded) return 0; @@ -2565,7 +2598,7 @@ int r600_vertex_elements_build_fetch_shader(struct r600_pipe_context *rctx, stru } memset(&bc, 0, sizeof(bc)); - r600_bytecode_init(&bc, rctx->chip_class); + r600_bytecode_init(&bc, rctx->chip_class, 0); for (i = 0; i < ve->count; i++) { if (elements[i].instance_divisor > 1) { diff --git a/src/gallium/drivers/r600/r600_asm.h b/src/gallium/drivers/r600/r600_asm.h index d0ff75d..a8e867c 100644 --- a/src/gallium/drivers/r600/r600_asm.h +++ b/src/gallium/drivers/r600/r600_asm.h @@ -176,6 +176,10 @@ struct r600_cf_callstack { int max; }; +#define AR_HANDLE_NORMAL 0 +#define AR_HANDLE_RV6XX 1 /* except RV670 */ + + struct r600_bytecode { enum chip_class chip_class; int type; @@ -194,13 +198,14 @@ struct r600_bytecode { struct r600_cf_callstackcallstack[SQ_MAX_CALL_DEPTH]; unsignedar_loaded; unsignedar_reg; + unsignedar_handling; }; /* eg_asm.c */ int eg_bytecode_cf_build(struct r600_bytecode *bc, struct r600_bytecode_cf *cf); /* r600_asm.c */ -void r600_bytecode_init(struct r600_bytecode *bc, enum chip_class chip_class); +void r600_bytecode_init(struct r600_bytecode *bc, enum chip_class chip_class, unsigned ar_handling); void r600_bytecode_clear(struct r600_bytecode *bc); int r600_bytecode_add_alu(struct r600_bytecode *bc, const struct r600_bytecode_alu *alu); int r600_bytecode_add_vtx(stru
[Mesa-dev] [PATCH] r600g: fix combined MEM_STREAM instructions
BURST_COUNT is clipped with ARRAY_SIZE, so set it to the max value to avoid clipping. Signed-off-by: Vadim Girlin --- src/gallium/drivers/r600/r600_shader.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c index 59d41cf..ce7f26b 100644 --- a/src/gallium/drivers/r600/r600_shader.c +++ b/src/gallium/drivers/r600/r600_shader.c @@ -1014,7 +1014,9 @@ static int r600_shader_from_tgsi(struct r600_pipe_context * rctx, struct r600_pi output.type = V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_WRITE; output.burst_count = 1; output.barrier = 1; - output.array_size = 0; + /* array_size is an upper limit for the burst_count +* with MEM_STREAM instructions */ + output.array_size = 0xFFF; output.comp_mask = (1 << so.output[i].num_components) - 1; if (ctx.bc->chip_class >= EVERGREEN) { switch (so.output[i].output_buffer) { -- 1.7.7.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: Set default access flags based on the run-time API
From: Ian Romanick The default access flags for OpenGL ES (via GL_OES_map_buffer) and desktop OpenGL are different. The code previously tried to handle this, but the decision was made at compile time. Since the same driver binary can be used for both OpenGL ES and desktop OpenGL, the decision must be made at run-time. This should fix bug #44433. It appears that the test case does various map and unmap operations and inspects the state of the buffer object around each. When it sees that GL_BUFFER_ACCESS does not match its expectations, it fails. NOTE: This is a candidate for release branches. Signed-off-by: Ian Romanick Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44433 --- v2: Convert DEFAULT_ACCESS macro to default_access_mode inline function. Add text from the relevant specs justifying the code. src/mesa/drivers/dri/intel/intel_buffer_objects.c |2 +- src/mesa/drivers/dri/nouveau/nouveau_bufferobj.c |2 +- .../drivers/dri/radeon/radeon_buffer_objects.c |2 +- src/mesa/main/bufferobj.c | 44 ++-- src/mesa/main/bufferobj.h |3 +- src/mesa/state_tracker/st_cb_bufferobjects.c |2 +- 6 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/mesa/drivers/dri/intel/intel_buffer_objects.c b/src/mesa/drivers/dri/intel/intel_buffer_objects.c index 03dd179..600f01c 100644 --- a/src/mesa/drivers/dri/intel/intel_buffer_objects.c +++ b/src/mesa/drivers/dri/intel/intel_buffer_objects.c @@ -72,7 +72,7 @@ intel_bufferobj_alloc(struct gl_context * ctx, GLuint name, GLenum target) { struct intel_buffer_object *obj = CALLOC_STRUCT(intel_buffer_object); - _mesa_initialize_buffer_object(&obj->Base, name, target); + _mesa_initialize_buffer_object(ctx, &obj->Base, name, target); obj->buffer = NULL; diff --git a/src/mesa/drivers/dri/nouveau/nouveau_bufferobj.c b/src/mesa/drivers/dri/nouveau/nouveau_bufferobj.c index dc5b152..f7ad895 100644 --- a/src/mesa/drivers/dri/nouveau/nouveau_bufferobj.c +++ b/src/mesa/drivers/dri/nouveau/nouveau_bufferobj.c @@ -56,7 +56,7 @@ nouveau_bufferobj_new(struct gl_context *ctx, GLuint buffer, GLenum target) if (!nbo) return NULL; - _mesa_initialize_buffer_object(&nbo->base, buffer, target); + _mesa_initialize_buffer_object(ctx, &nbo->base, buffer, target); return &nbo->base; } diff --git a/src/mesa/drivers/dri/radeon/radeon_buffer_objects.c b/src/mesa/drivers/dri/radeon/radeon_buffer_objects.c index 7b59c03..5abc52b 100644 --- a/src/mesa/drivers/dri/radeon/radeon_buffer_objects.c +++ b/src/mesa/drivers/dri/radeon/radeon_buffer_objects.c @@ -46,7 +46,7 @@ radeonNewBufferObject(struct gl_context * ctx, { struct radeon_buffer_object *obj = CALLOC_STRUCT(radeon_buffer_object); -_mesa_initialize_buffer_object(&obj->Base, name, target); +_mesa_initialize_buffer_object(ctx, &obj->Base, name, target); obj->bo = NULL; diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index 5f8071f..5b6db78 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -49,13 +49,6 @@ /*#define BOUNDS_CHECK*/ -#if FEATURE_OES_mapbuffer -#define DEFAULT_ACCESS GL_MAP_WRITE_BIT -#else -#define DEFAULT_ACCESS (GL_MAP_READ_BIT | GL_MAP_WRITE_BIT) -#endif - - /** * Used as a placeholder for buffer objects between glGenBuffers() and * glBindBuffer() so that glIsBuffer() can work correctly. @@ -122,6 +115,30 @@ get_buffer(struct gl_context *ctx, GLenum target) } +static inline GLenum +default_access_mode(const struct gl_context *ctx) +{ + /* Table 2.6 on page 31 (page 44 of the PDF) of the OpenGL 1.5 spec says: +* +* Name Type Initial Value Legal Values +* ...... ...... +* BUFFER_ACCESS enum READ_WRITE READ_ONLY, WRITE_ONLY +* READ_WRITE +* +* However, table 6.8 in the GL_OES_mapbuffer extension says: +* +* Get Value Type Get Command Value Description +* - --- - --- +* BUFFER_ACCESS_OES Z1 GetBufferParameteriv WRITE_ONLY_OES buffer map flag +* +* The difference is because GL_OES_mapbuffer only supports mapping buffers +* write-only. +*/ + return (ctx->API == API_OPENGLES) + ? GL_MAP_WRITE_BIT : (GL_MAP_READ_BIT | GL_MAP_WRITE_BIT); +} + + /** * Convert a GLbitfield describing the mapped buffer access flags * into one of GL_READ_WRITE, GL_READ_ONLY, or GL_WRITE_ONLY. @@ -213,7 +230,7 @@ _mesa_new_buffer_object( struct gl_context *ctx, GLuint name, GLenum target ) (void) ctx; obj = MALLOC_STRUCT(gl_buffer_object); - _mesa_initialize_buffer_object(obj, name, target); + _mesa_initialize_buffer_object(ctx, obj, name, target); return obj; } @@ -311,7 +328,8 @@ _mesa_reference_buffer_object_(struct gl_conte
Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO
On 01/18/2012 02:23 AM, Yuanhan Liu wrote: When rendering to FBO, rendering is inverted. At the same time, we would also make sure the point sprite origin is inverted. Or, we will get an inverted result correspoinding to rendering to the default winsys FBO. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613 Signed-off-by: Yuanhan Liu --- src/mesa/drivers/dri/i965/brw_defines.h |1 + src/mesa/drivers/dri/i965/gen6_sf_state.c | 19 +-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h index 4d90a99..029be87 100644 --- a/src/mesa/drivers/dri/i965/brw_defines.h +++ b/src/mesa/drivers/dri/i965/brw_defines.h @@ -1128,6 +1128,7 @@ enum brw_message_target { /* DW1 (for gen6) */ # define GEN6_SF_NUM_OUTPUTS_SHIFT22 # define GEN6_SF_SWIZZLE_ENABLE (1<< 21) +# define GEN6_SF_POINT_SPRITE_UPPERLEFT(0<< 20) # define GEN6_SF_POINT_SPRITE_LOWERLEFT (1<< 20) # define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT 11 # define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT 4 diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c index 548c5a3..d354a2b 100644 --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c I think gen7_sf_state.c needs similar changes. @@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw) float point_size; uint16_t attr_overrides[FRAG_ATTRIB_MAX]; bool userclip_active; + int point_sprite_origin; dw1 is a uint32_t, so this should be too. /* _NEW_TRANSFORM */ userclip_active = (ctx->Transform.ClipPlanesEnabled != 0); @@ -258,8 +259,22 @@ upload_sf_state(struct brw_context *brw) /* Clamp to the hardware limits and convert to fixed point */ dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3); - if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT) - dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT; + /* +* When rendering to FBO, rendering is inverted. At the same time, +* we would also make sure the point sprite origin is inverted. +* Or, we will get an inverted result corresponding to rendering +* to the default/window FBO. +*/ + if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT) { + point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT; + if (render_to_fbo) + point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT; + } else { + point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT; + if (render_to_fbo) + point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT; + } + dw1 |= point_sprite_origin; /* _NEW_LIGHT */ if (ctx->Light.ProvokingVertex != GL_FIRST_VERTEX_CONVENTION) { ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] renderbuffer-cleanups-v2 branch
On 01/17/2012 02:58 PM, Brian Paul wrote: On 01/16/2012 08:30 PM, Brian Paul wrote: On Mon, Jan 16, 2012 at 4:31 PM, Ian Romanick wrote: On 01/16/2012 01:30 PM, Brian Paul wrote: The renderbuffer-cleanups-v2 branch removes all the old swrast GetRow/PutRow stuff. All swrast rendering is now done through renderbuffer mapping and the format_pack/unpack.c code. Woo hoo! Thanks for tackling this! I've been slowly putting bandaids on some of the code, but it looks like you've saved me a ton of work! The gl_renderbuffer type is smaller and cleaner now. Plus, a few more old driver hooks are removed. I'm reviewing this code, and it looks good so far. However, it looks like it won't build at every commit. That's really important for bisecting. For example, I notice the patch that adds uses of gl_renderbuffer::Map appear before the patch that adds the field. Hmm, I'll have to check on that tomorrow. I redid the whole patch series today (hence -v2) and could have made a mistake along the way. I rebuilt at a few intermediate points and didn't see any issues. I tried your check_all_commits.sh script but it didn't work for me: $ ~/check_all_commits.sh . build2/ f2749c52627aa1a45cbd8ee0dbbe460fbf1aeb36..496 e7a2986a5abc82c982a3994b3e46d21e5a4b0 The script moves around a lot, so it doesn't like . as a directory. I'll give it a try, but my guess is that using $(pwd) would have worked. The script is also really specific to my build environment, so it may need some other hacking to work for you anyway. /home/brian/check_all_commits.sh: 24: function: not found /home/brian/check_all_commits.sh: 37: cannot create /tmp/warn-master-.-build2/.t xt: Directory nonexistent grep: /tmp/build-master-.-build2/.txt: No such file or directory /home/brian/check_all_commits.sh: 37: cannot create /tmp/warn-f2749c52627aa1a45c bd8ee0dbbe460fbf1aeb36..496e7a2986a5abc82c982a3994b3e46d21e5a4b0-.-build2/.txt: Directory nonexistent grep: /tmp/build-f2749c52627aa1a45cbd8ee0dbbe460fbf1aeb36..496e7a2986a5abc82c982 a3994b3e46d21e5a4b0-.-build2/.txt: No such file or directory Warning changes in f2749c52627aa1a45cbd8ee0dbbe460fbf1aeb36..496e7a2986a5abc82c9 82a3994b3e46d21e5a4b0 (.-bit, build2/): diff: /tmp/warn-master-.-build2/.txt: No such file or directory diff: /tmp/warn-f2749c52627aa1a45cbd8ee0dbbe460fbf1aeb36..496e7a2986a5abc82c982a 3994b3e46d21e5a4b0-.-build2/.txt: No such file or directory Installation directory does not exist. Maybe you could run the script and let me know if/where there is a bad commit. I'll do that today. On 54 commits, it will take some time. :) -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO
On 01/18/2012 02:21 AM, Yuanhan Liu wrote: On Wed, Jan 18, 2012 at 06:23:52PM +0800, Yuanhan Liu wrote: When rendering to FBO, rendering is inverted. At the same time, we would also make sure the point sprite origin is inverted. Or, we will get an inverted result correspoinding to rendering to the default winsys FBO. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613 Actually, I'm not sure about this patch, and I would like to treat this patch as a _workaround_. So, any comments are welcome. Why? Do you have a different solution in mind? This patch fixes this problem the way I would have expected it to be fixed. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Set default access flags based on the run-time API
On 01/18/2012 12:41 PM, Ian Romanick wrote: From: Ian Romanick The default access flags for OpenGL ES (via GL_OES_map_buffer) and desktop OpenGL are different. The code previously tried to handle this, but the decision was made at compile time. Since the same driver binary can be used for both OpenGL ES and desktop OpenGL, the decision must be made at run-time. This should fix bug #44433. It appears that the test case does various map and unmap operations and inspects the state of the buffer object around each. When it sees that GL_BUFFER_ACCESS does not match its expectations, it fails. NOTE: This is a candidate for release branches. Signed-off-by: Ian Romanick Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44433 --- v2: Convert DEFAULT_ACCESS macro to default_access_mode inline function. Add text from the relevant specs justifying the code. Reviewed-by: Brian Paul ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: Loosen glBlitFramebuffer restrictions on depthstencil buffers (v2)
--- snip Supporting Z32 -> Z32_FLOAT is a bad idea. So I've amended the patch to avoid that. --- end snip This loosens the format validation in glBlitFramebuffer. When blitting depth bits, don't require an exact match between the depth formats; only require that the two formats have the same number of depth bits and the same depth datatype (float vs uint). Ditto for stencil. Between S8_Z24 buffers, the EXT_framebuffer_blit spec allows glBlitFramebuffer to blit the depth and stencil bits separately. So I see no reason to prevent blitting the depth bits between X8_Z24 and S8_Z24 or the stencil bits between S8 and S8_Z24. However, we of course don't want to allow blitting from Z32 to Z32_FLOAT. Fixes Piglit fbo/fbo-blit-d24s8 on Intel drivers with separate stencil enabled. The problem was that, on Intel drivers with separate stencil, the default framebuffer has separate depth and stencil buffers with formats X8_Z24 and S8. The test attempts to blit the depth bits from a S8_Z24 buffer into the default framebuffer. v2: Check that depth datatypes match in order prevent Z32 -> Z32_FLOAT. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44665 Note: This is a candidate for the 8.0 branch. CC: Brian Paul CC: Kenneth Graunke CC: Iam Romanick Reported-by: Xunx Fang Signed-off-by: Chad Versace --- src/mesa/main/fbobject.c | 13 ++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index 0524959..2b3ac2e 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -2709,9 +2709,13 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, if ((readRb == NULL) || (drawRb == NULL)) { mask &= ~GL_STENCIL_BUFFER_BIT; } - else if (readRb->Format != drawRb->Format) { + else if (_mesa_get_format_bits(readRb->Format, GL_STENCIL_BITS) != + _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) { +/* There is no need to check the stencil datatype here, because + * there is only one: GL_UNSIGNED_INT. + */ _mesa_error(ctx, GL_INVALID_OPERATION, - "glBlitFramebufferEXT(stencil buffer format mismatch)"); + "glBlitFramebufferEXT(stencil buffer size mismatch)"); return; } } @@ -2731,7 +2735,10 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, if ((readRb == NULL) || (drawRb == NULL)) { mask &= ~GL_DEPTH_BUFFER_BIT; } - else if (readRb->Format != drawRb->Format) { + else if ((_mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS) != + _mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS)) || + (_mesa_get_format_datatype(readRb->Format) != + _mesa_get_format_datatype(drawRb->Format))) { _mesa_error(ctx, GL_INVALID_OPERATION, "glBlitFramebufferEXT(depth buffer format mismatch)"); return; -- 1.7.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: Fix isinf() for non-C99-compliant compilers.
Commit ede60bc4670a8d9c14921c77abee1ac57fc0e6bf (glsl: Add isinf() and isnan() builtins) uses "+INF" in the .ir file to represent infinity. This worked on C99-compliant compilers, since the s-expression reader uses strtod() to read numbers, and C99 requires strtod() to understand "+INF". However, it didn't work on non-C99-compliant compilers such as MSVC. This patch modifies the s-expression reader to explicitly check for "+INF" rather than relying on strtod() to support it. This is a candidate for the 8.0 branch. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44767 Tested-by: Morgan Armand --- src/glsl/s_expression.cpp | 36 ++-- 1 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/glsl/s_expression.cpp b/src/glsl/s_expression.cpp index e704a3b..57de9d3 100644 --- a/src/glsl/s_expression.cpp +++ b/src/glsl/s_expression.cpp @@ -23,6 +23,7 @@ */ #include +#include #include "s_expression.h" s_symbol::s_symbol(const char *str, size_t n) @@ -64,21 +65,28 @@ read_atom(void *ctx, const char *&src, char *&symbol_buffer) if (n == 0) return NULL; // no atom - // Check if the atom is a number. - char *float_end = NULL; - double f = glsl_strtod(src, &float_end); - if (float_end != src) { - char *int_end = NULL; - int i = strtol(src, &int_end, 10); - // If strtod matched more characters, it must have a decimal part - if (float_end > int_end) -expr = new(ctx) s_float(f); - else -expr = new(ctx) s_int(i); + // Check for the special symbol '+INF', which means +Infinity. Note: C99 + // requires strtod to parse '+INF' as +Infinity, but we still support some + // non-C99-compliant compilers (e.g. MSVC). + if (n == 4 && strncmp(src, "+INF", 4) == 0) { + expr = new(ctx) s_float(std::numeric_limits::infinity()); } else { - // Not a number; return a symbol. - symbol_buffer[n] = '\0'; - expr = new(ctx) s_symbol(symbol_buffer, n); + // Check if the atom is a number. + char *float_end = NULL; + double f = glsl_strtod(src, &float_end); + if (float_end != src) { + char *int_end = NULL; + int i = strtol(src, &int_end, 10); + // If strtod matched more characters, it must have a decimal part + if (float_end > int_end) +expr = new(ctx) s_float(f); + else +expr = new(ctx) s_int(i); + } else { + // Not a number; return a symbol. + symbol_buffer[n] = '\0'; + expr = new(ctx) s_symbol(symbol_buffer, n); + } } src += n; -- 1.7.6.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 44912] New: [bisected] WebGL conformance/textures/texture-mips tests fails
https://bugs.freedesktop.org/show_bug.cgi?id=44912 Bug #: 44912 Summary: [bisected] WebGL conformance/textures/texture-mips tests fails Classification: Unclassified Product: Mesa Version: git Platform: Other URL: https://cvs.khronos.org/svn/repos/registry/trunk/publi c/webgl/sdk/tests/webgl-conformance-tests.html OS/Version: All Status: NEW Keywords: regression Severity: normal Priority: medium Component: Mesa core AssignedTo: mesa-dev@lists.freedesktop.org ReportedBy: pavel.ondra...@email.cz Previously all 19 subtests passed, now only 17 or 18 (random) pass. This is with latest mesa, r300g driver (RV530), Firefox 9.0.1 https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/textures/texture-mips.html 5d67d4fbebb7c7d6582c4c92fc5cea4a8e6a60ab is the first bad commit commit 5d67d4fbebb7c7d6582c4c92fc5cea4a8e6a60ab Author: Brian Paul Date: Wed Jan 4 13:30:35 2012 -0700 st/mesa: remove st_TexImage(), use core Mesa code instead The core Mesa code does the equivalent memory allocation, image mapping, storing and unmapping. We just need to call prep_teximage() first to handle the 'surface_based' stuff. The other change is to always use the level=0 mipmap image when accessing individual mipmap level images that are stored in resources/buffers. Apparently, we were always using malloc'd memory for individual mipmap images, not resource buffers, before. Signed-off-by: Brian Paul -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- 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] glsl: Fix isinf() for non-C99-compliant compilers.
Looks good to me. Thanks Should there be a case for -INF while we are at it? Jose - Original Message - > Commit ede60bc4670a8d9c14921c77abee1ac57fc0e6bf (glsl: Add isinf() > and > isnan() builtins) uses "+INF" in the .ir file to represent infinity. > This worked on C99-compliant compilers, since the s-expression reader > uses strtod() to read numbers, and C99 requires strtod() to > understand > "+INF". However, it didn't work on non-C99-compliant compilers such > as MSVC. > > This patch modifies the s-expression reader to explicitly check for > "+INF" rather than relying on strtod() to support it. > > This is a candidate for the 8.0 branch. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44767 > Tested-by: Morgan Armand > --- > src/glsl/s_expression.cpp | 36 > ++-- > 1 files changed, 22 insertions(+), 14 deletions(-) > > diff --git a/src/glsl/s_expression.cpp b/src/glsl/s_expression.cpp > index e704a3b..57de9d3 100644 > --- a/src/glsl/s_expression.cpp > +++ b/src/glsl/s_expression.cpp > @@ -23,6 +23,7 @@ > */ > > #include > +#include > #include "s_expression.h" > > s_symbol::s_symbol(const char *str, size_t n) > @@ -64,21 +65,28 @@ read_atom(void *ctx, const char *&src, char > *&symbol_buffer) > if (n == 0) >return NULL; // no atom > > - // Check if the atom is a number. > - char *float_end = NULL; > - double f = glsl_strtod(src, &float_end); > - if (float_end != src) { > - char *int_end = NULL; > - int i = strtol(src, &int_end, 10); > - // If strtod matched more characters, it must have a decimal > part > - if (float_end > int_end) > - expr = new(ctx) s_float(f); > - else > - expr = new(ctx) s_int(i); > + // Check for the special symbol '+INF', which means +Infinity. > Note: C99 > + // requires strtod to parse '+INF' as +Infinity, but we still > support some > + // non-C99-compliant compilers (e.g. MSVC). > + if (n == 4 && strncmp(src, "+INF", 4) == 0) { > + expr = new(ctx) > s_float(std::numeric_limits::infinity()); > } else { > - // Not a number; return a symbol. > - symbol_buffer[n] = '\0'; > - expr = new(ctx) s_symbol(symbol_buffer, n); > + // Check if the atom is a number. > + char *float_end = NULL; > + double f = glsl_strtod(src, &float_end); > + if (float_end != src) { > + char *int_end = NULL; > + int i = strtol(src, &int_end, 10); > + // If strtod matched more characters, it must have a > decimal part > + if (float_end > int_end) > +expr = new(ctx) s_float(f); > + else > +expr = new(ctx) s_int(i); > + } else { > + // Not a number; return a symbol. > + symbol_buffer[n] = '\0'; > + expr = new(ctx) s_symbol(symbol_buffer, n); > + } > } > > src += n; > -- > 1.7.6.5 > > ___ > 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] [Nouveau] [PATCH 4/4] nvfx: random cleanups of the state validation code
Le Tue, 17 Jan 2012 18:22:59 +0100 Patrice Mandin a écrit: > Le Tue, 10 Jan 2012 12:41:04 +0100 > Lucas Stach a écrit: > > > Signed-off-by: Lucas Stach > > --- > > src/gallium/drivers/nvfx/nvfx_state_emit.c | 49 > > --- > > 1 files changed, 22 insertions(+), 27 deletions(-) > > > > diff --git a/src/gallium/drivers/nvfx/nvfx_state_emit.c > > b/src/gallium/drivers/nvfx/nvfx_state_emit.c > > index e2cfb76..a959015 100644 > [snip] @@ -212,14 +211,6 @@ nvfx_state_validate_common(struct nvfx_context *nvfx) nvfx->relocs_needed = NVFX_RELOCATE_ALL; } - if(nvfx->dirty & NVFX_NEW_SAMPLER) { - nvfx->dirty &=~ NVFX_NEW_SAMPLER; - nvfx_fragtex_validate(nvfx); - - // TODO: only set this if really necessary - flush_tex_cache = TRUE; - } - dirty = nvfx->dirty; if(nvfx->render_mode == HW) @@ -252,6 +243,13 @@ nvfx_state_validate_common(struct nvfx_context *nvfx) } } + if(dirty & NVFX_NEW_SAMPLER) { + nvfx_fragtex_validate(nvfx); + + // TODO: only set this if really necessary + flush_tex_cache = TRUE; + } + if(dirty & NVFX_NEW_RAST) sb_emit(chan, nvfx->rasterizer->sb, nvfx->rasterizer->sb_len); > I noticed another small regression with it, the splashscreen bitmap is > also not displayed anymore, so I'll investigate a bit further. Maybe no > need to revert the whole series. And the above part triggers it, the missing nvfx->dirty &=~ NVFX_NEW_SAMPLER; in the change causes this regression. -- Patrice Mandin WWW: http://pmandin.atari.org/ Programmeur Linux, Atari Spécialité: Développement, jeux "who writes the code, decides" ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Fix isinf() for non-C99-compliant compilers.
On 18 January 2012 12:43, Jose Fonseca wrote: > Looks good to me. Thanks > > Should there be a case for -INF while we are at it? > I think you can make arguments both for and against. On the one hand, C99 requires strtod to recognize "INF", "+INF", "-INF", "INFINITY", "+INFINITY", "-INFINITY", and capitalization variations of those, in addition to a bunch of representations of NaN, so for maximum portability you might argue we should make our s-expression parser handle all of those even on non-C99 systems. On the other hand, s-expression parsing is only used for builtin functions, and at the moment builtin functions only use "+INF", so any code we write to support other forms won't actually be exercised. Personally, I favor just doing "+INF" on the grounds that it is the smallest fix that meets our present needs, and that adding anything else would require extra testing effort to validate. Keeping the fix as small as possible seems especially prudent considering that this fix is a candidate for the 8.0 branch. Paul ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] r600g: fixup AR handling (v3)
From: Dave Airlie So it appears R600s (except rv670) do AR handling different using a different opcode. This patch fixes up r600g to work properly on r600. This fixes ~100 piglit tests here (in GLSL1.30 mode) on rv610. v3: add index_mode as per the docs. This still fails any dst relative tests for some reason I can't quite see yet, but it passes a lot more tests than without. Signed-off-by: Dave Airlie --- src/gallium/drivers/r600/r600_asm.c| 48 --- src/gallium/drivers/r600/r600_asm.h|8 - src/gallium/drivers/r600/r600_shader.c |6 +++- src/gallium/drivers/r600/r600_sq.h |7 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/gallium/drivers/r600/r600_asm.c b/src/gallium/drivers/r600/r600_asm.c index 8234744..aad286b 100644 --- a/src/gallium/drivers/r600/r600_asm.c +++ b/src/gallium/drivers/r600/r600_asm.c @@ -94,6 +94,7 @@ static inline unsigned int r600_bytecode_get_num_operands(struct r600_bytecode * case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOV: case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA: case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_FLOOR: + case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT: case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_INT: case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FRACT: case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLOOR: @@ -249,10 +250,11 @@ static struct r600_bytecode_tex *r600_bytecode_tex(void) return tex; } -void r600_bytecode_init(struct r600_bytecode *bc, enum chip_class chip_class) +void r600_bytecode_init(struct r600_bytecode *bc, enum chip_class chip_class, unsigned ar_handling) { LIST_INITHEAD(&bc->cf); bc->chip_class = chip_class; + bc->ar_handling = ar_handling; } static int r600_bytecode_add_cf(struct r600_bytecode *bc) @@ -441,7 +443,8 @@ static int is_alu_mova_inst(struct r600_bytecode *bc, struct r600_bytecode_alu * return !alu->is_op3 && ( alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA || alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_FLOOR || - alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_INT); + alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_INT || + alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT); case EVERGREEN: case CAYMAN: default: @@ -457,7 +460,8 @@ static int is_alu_vec_unit_inst(struct r600_bytecode *bc, struct r600_bytecode_a case R600: case R700: return is_alu_reduction_inst(bc, alu) || - is_alu_mova_inst(bc, alu); + (is_alu_mova_inst(bc, alu) && +(alu->inst != V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT)); case EVERGREEN: case CAYMAN: default: @@ -478,6 +482,7 @@ static int is_alu_trans_unit_inst(struct r600_bytecode *bc, struct r600_bytecode case R700: if (!alu->is_op3) return alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_ASHR_INT || + alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT || alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_INT_TO_FLT || alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLT_TO_UINT || alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLT_TO_INT || @@ -1236,12 +1241,43 @@ static int r600_bytecode_alloc_kcache_lines(struct r600_bytecode *bc, struct r60 return 0; } + +/* load AR register from gpr (bc->ar_reg) with MOVA_INT */ +static int load_ar_r6xx(struct r600_bytecode *bc) +{ + struct r600_bytecode_alu alu; + int r; + + if (bc->ar_loaded) + return 0; + + /* hack to avoid making MOVA the last instruction in the clause */ + if ((bc->cf_last->ndw>>1) >= 110) + bc->force_add_cf = 1; + + memset(&alu, 0, sizeof(alu)); + alu.inst = V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT; + alu.src[0].sel = bc->ar_reg; + alu.last = 1; + alu.index_mode = INDEX_MODE_LOOP; + r = r600_bytecode_add_alu(bc, &alu); + if (r) + return r; + + bc->cf_last->r6xx_uses_waterfall = 1; + bc->ar_loaded = 1; + return 0; +} + /* load AR register from gpr (bc->ar_reg) with MOVA_INT */ static int load_ar(struct r600_bytecode *bc) { struct r600_bytecode_alu alu; int r; + if (bc->ar_handling) + return load_ar_r6xx(bc); + if (bc->ar_loaded) return 0; @@ -1599,6 +1635,7 @@ static int r600_bytecode_alu_build(struct r600_bytecode *bc, struct r600_bytecod S_SQ_ALU_WORD0_SRC1_REL(alu->src[1].rel) | S_SQ_ALU_WORD0_SRC1_CHAN(alu->src[1].
Re: [Mesa-dev] [PATCH] r600g: fix streamout on evergreen
Well spotted, thanks! Reviewed-by: Marek Olšák Marek On Wed, Jan 18, 2012 at 10:46 AM, Vadim Girlin wrote: > Enable it in the evergreen_context_draw if needed. > Same as already done in the r600_context_draw for r6xx/r7xx. > > Signed-off-by: Vadim Girlin > --- > src/gallium/drivers/r600/evergreen_hw_context.c | 6 ++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/src/gallium/drivers/r600/evergreen_hw_context.c > b/src/gallium/drivers/r600/evergreen_hw_context.c > index e75eaf2..9401d82 100644 > --- a/src/gallium/drivers/r600/evergreen_hw_context.c > +++ b/src/gallium/drivers/r600/evergreen_hw_context.c > @@ -1168,6 +1168,12 @@ void evergreen_context_draw(struct r600_context *ctx, > const struct r600_draw *dr > r600_context_block_resource_emit_dirty(ctx, dirty_block); > } > > + /* Enable stream out if needed. */ > + if (ctx->streamout_start) { > + r600_context_streamout_begin(ctx); > + ctx->streamout_start = FALSE; > + } > + > /* draw packet */ > pm4 = &ctx->pm4[ctx->pm4_cdwords]; > pm4[0] = PKT3(PKT3_INDEX_TYPE, 0, ctx->predicate_drawing); > -- > 1.7.7.5 > > ___ > 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 2/2] i965/vs: Enable workaround-free math on gen7.
This is similar to a commit that did the same for the FS. Shaves several more instructions off of the VS in Lightsmark, but no statistically significant performance difference (n=5). --- src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp index 2436bc9..898e78d 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp @@ -313,7 +313,9 @@ vec4_visitor::emit_math(opcode opcode, dst_reg dst, src_reg src) return; } - if (intel->gen >= 6) { + if (intel->gen >= 7) { + emit(opcode, dst, src); + } else if (intel->gen == 6) { return emit_math1_gen6(opcode, dst, src); } else { return emit_math1_gen4(opcode, dst, src); @@ -380,7 +382,9 @@ vec4_visitor::emit_math(enum opcode opcode, return; } - if (intel->gen >= 6) { + if (intel->gen >= 7) { + emit(opcode, dst, src0, src1); + } else if (intel->gen == 6) { return emit_math2_gen6(opcode, dst, src0, src1); } else { return emit_math2_gen4(opcode, dst, src0, src1); -- 1.7.7.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] i965/vs: Use the embedded-comparison SEL on gen6+, like the FS does.
Shaves a few instructions off of the VS in Lightsmark, but no statistically significant performance difference on gen7 (n=5). --- src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 22 -- 1 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp index 06bde92..2436bc9 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp @@ -1285,16 +1285,26 @@ vec4_visitor::visit(ir_expression *ir) break; case ir_binop_min: - emit(CMP(result_dst, op[0], op[1], BRW_CONDITIONAL_L)); + if (intel->gen >= 6) { +inst = emit(BRW_OPCODE_SEL, result_dst, op[0], op[1]); +inst->conditional_mod = BRW_CONDITIONAL_L; + } else { +emit(CMP(result_dst, op[0], op[1], BRW_CONDITIONAL_L)); - inst = emit(BRW_OPCODE_SEL, result_dst, op[0], op[1]); - inst->predicate = BRW_PREDICATE_NORMAL; +inst = emit(BRW_OPCODE_SEL, result_dst, op[0], op[1]); +inst->predicate = BRW_PREDICATE_NORMAL; + } break; case ir_binop_max: - emit(CMP(result_dst, op[0], op[1], BRW_CONDITIONAL_G)); + if (intel->gen >= 6) { +inst = emit(BRW_OPCODE_SEL, result_dst, op[0], op[1]); +inst->conditional_mod = BRW_CONDITIONAL_G; + } else { +emit(CMP(result_dst, op[0], op[1], BRW_CONDITIONAL_G)); - inst = emit(BRW_OPCODE_SEL, result_dst, op[0], op[1]); - inst->predicate = BRW_PREDICATE_NORMAL; +inst = emit(BRW_OPCODE_SEL, result_dst, op[0], op[1]); +inst->predicate = BRW_PREDICATE_NORMAL; + } break; case ir_binop_pow: -- 1.7.7.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Fix isinf() for non-C99-compliant compilers.
On 01/18/2012 12:32 PM, Paul Berry wrote: Commit ede60bc4670a8d9c14921c77abee1ac57fc0e6bf (glsl: Add isinf() and isnan() builtins) uses "+INF" in the .ir file to represent infinity. This worked on C99-compliant compilers, since the s-expression reader uses strtod() to read numbers, and C99 requires strtod() to understand "+INF". However, it didn't work on non-C99-compliant compilers such as MSVC. This patch modifies the s-expression reader to explicitly check for "+INF" rather than relying on strtod() to support it. This is a candidate for the 8.0 branch. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44767 Tested-by: Morgan Armand Ah, C89, how I hate thee... I'm in favor of only reading "+INF" for now. This looks acceptable. Reviewed-by: Kenneth Graunke ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] r600g: fixup AR handling (v3)
On Wed, 2012-01-18 at 21:49 +, Dave Airlie wrote: > From: Dave Airlie > > So it appears R600s (except rv670) do AR handling different using a different > opcode. This patch fixes up r600g to work properly on r600. > > This fixes ~100 piglit tests here (in GLSL1.30 mode) on rv610. > > v3: add index_mode as per the docs. > > This still fails any dst relative tests for some reason I can't quite see yet, > but it passes a lot more tests than without. I guess it's the problem described in the r6xx_r7xx_3d.pdf: "6.1.4 Shader GPR Indexing may return incorrect result This affects R600, RV630 and RV610, but not RV670 or RS780. ... MOV R[A0.x +2], R33 ADD R20, R20, R2 // H/w thinks R2 is the same as the prev dest and will substitute PV" Vadim > > Signed-off-by: Dave Airlie > --- > src/gallium/drivers/r600/r600_asm.c| 48 --- > src/gallium/drivers/r600/r600_asm.h|8 - > src/gallium/drivers/r600/r600_shader.c |6 +++- > src/gallium/drivers/r600/r600_sq.h |7 > 4 files changed, 62 insertions(+), 7 deletions(-) > > diff --git a/src/gallium/drivers/r600/r600_asm.c > b/src/gallium/drivers/r600/r600_asm.c > index 8234744..aad286b 100644 > --- a/src/gallium/drivers/r600/r600_asm.c > +++ b/src/gallium/drivers/r600/r600_asm.c > @@ -94,6 +94,7 @@ static inline unsigned int > r600_bytecode_get_num_operands(struct r600_bytecode * > case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOV: > case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA: > case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_FLOOR: > + case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT: > case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_INT: > case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FRACT: > case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLOOR: > @@ -249,10 +250,11 @@ static struct r600_bytecode_tex *r600_bytecode_tex(void) > return tex; > } > > -void r600_bytecode_init(struct r600_bytecode *bc, enum chip_class chip_class) > +void r600_bytecode_init(struct r600_bytecode *bc, enum chip_class > chip_class, unsigned ar_handling) > { > LIST_INITHEAD(&bc->cf); > bc->chip_class = chip_class; > + bc->ar_handling = ar_handling; > } > > static int r600_bytecode_add_cf(struct r600_bytecode *bc) > @@ -441,7 +443,8 @@ static int is_alu_mova_inst(struct r600_bytecode *bc, > struct r600_bytecode_alu * > return !alu->is_op3 && ( > alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA || > alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_FLOOR > || > - alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_INT); > + alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_INT || > + alu->inst == > V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT); > case EVERGREEN: > case CAYMAN: > default: > @@ -457,7 +460,8 @@ static int is_alu_vec_unit_inst(struct r600_bytecode *bc, > struct r600_bytecode_a > case R600: > case R700: > return is_alu_reduction_inst(bc, alu) || > - is_alu_mova_inst(bc, alu); > + (is_alu_mova_inst(bc, alu) && > + (alu->inst != > V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT)); > case EVERGREEN: > case CAYMAN: > default: > @@ -478,6 +482,7 @@ static int is_alu_trans_unit_inst(struct r600_bytecode > *bc, struct r600_bytecode > case R700: > if (!alu->is_op3) > return alu->inst == > V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_ASHR_INT || > + alu->inst == > V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT || > alu->inst == > V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_INT_TO_FLT || > alu->inst == > V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLT_TO_UINT || > alu->inst == > V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLT_TO_INT || > @@ -1236,12 +1241,43 @@ static int r600_bytecode_alloc_kcache_lines(struct > r600_bytecode *bc, struct r60 > return 0; > } > > + > +/* load AR register from gpr (bc->ar_reg) with MOVA_INT */ > +static int load_ar_r6xx(struct r600_bytecode *bc) > +{ > + struct r600_bytecode_alu alu; > + int r; > + > + if (bc->ar_loaded) > + return 0; > + > + /* hack to avoid making MOVA the last instruction in the clause */ > + if ((bc->cf_last->ndw>>1) >= 110) > + bc->force_add_cf = 1; > + > + memset(&alu, 0, sizeof(alu)); > + alu.inst = V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT; > + alu.src[0].sel = bc->ar_reg; > + alu.last = 1; > + alu.index_mode = INDEX_MODE_LOOP; > + r = r600_bytecode_add_alu(bc, &alu); > + if (r) > + return r; > + > + bc->cf_last->r6xx_uses_waterfall = 1; > + bc->ar_loaded = 1; > + return 0; > +} > + > /* load
Re: [Mesa-dev] [Nouveau] [PATCH 4/4] nvfx: random cleanups of the state validation code
Am Mittwoch, den 18.01.2012, 21:54 +0100 schrieb Patrice Mandin: > Le Tue, 17 Jan 2012 18:22:59 +0100 > Patrice Mandin a écrit: > > > Le Tue, 10 Jan 2012 12:41:04 +0100 > > Lucas Stach a écrit: > > > > > Signed-off-by: Lucas Stach > > > --- > > > src/gallium/drivers/nvfx/nvfx_state_emit.c | 49 > > > --- > > > 1 files changed, 22 insertions(+), 27 deletions(-) > > > > > > diff --git a/src/gallium/drivers/nvfx/nvfx_state_emit.c > > > b/src/gallium/drivers/nvfx/nvfx_state_emit.c > > > index e2cfb76..a959015 100644 > > [snip] > @@ -212,14 +211,6 @@ nvfx_state_validate_common(struct nvfx_context *nvfx) > nvfx->relocs_needed = NVFX_RELOCATE_ALL; > } > > - if(nvfx->dirty & NVFX_NEW_SAMPLER) { > - nvfx->dirty &=~ NVFX_NEW_SAMPLER; > - nvfx_fragtex_validate(nvfx); > - > - // TODO: only set this if really necessary > - flush_tex_cache = TRUE; > - } > - > dirty = nvfx->dirty; > > if(nvfx->render_mode == HW) > @@ -252,6 +243,13 @@ nvfx_state_validate_common(struct nvfx_context *nvfx) > } > } > > + if(dirty & NVFX_NEW_SAMPLER) { > + nvfx_fragtex_validate(nvfx); > + > + // TODO: only set this if really necessary > + flush_tex_cache = TRUE; > + } > + > if(dirty & NVFX_NEW_RAST) > sb_emit(chan, nvfx->rasterizer->sb, nvfx->rasterizer->sb_len); > > > I noticed another small regression with it, the splashscreen bitmap is > > also not displayed anymore, so I'll investigate a bit further. Maybe no > > need to revert the whole series. > > And the above part triggers it, the missing nvfx->dirty &=~ NVFX_NEW_SAMPLER; > in the change causes this regression. > So the supposedly low risk cleanups broke all of those things. Thanks to take the time to track this down to this level. Will fix it up soon. Thanks, Lucas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] autoconf: Fix build of dri symbols test to not manually link expat.
AC_CHECK_LIB has this nasty behavior, like the cflags tests, of automatically putting the tested value into the global LIBS on success. This caused -lexpat to end up in LIBS, but without the --with-expat dir, so my 32-bit build on a 64 system using expat from a custom prefix could only find the system expat and fail to link on the one current consumer of the LIBS variable: the dri driver test link. --- configure.ac |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index f3f5e3e..7baa04a 100644 --- a/configure.ac +++ b/configure.ac @@ -1269,8 +1269,10 @@ if test "x$enable_dri" = xyes; then EXPAT_LIB="-L$withval/$LIB_DIR -lexpat" ]) AC_CHECK_HEADER([expat.h],[],[AC_MSG_ERROR([Expat required for DRI.])]) + save_LIBS="$LIBS" AC_CHECK_LIB([expat],[XML_ParserCreate],[], [AC_MSG_ERROR([Expat required for DRI.])]) + LIBS="$save_LIBS" fi # libdrm is required for all except swrast -- 1.7.7.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO
On Wed, Jan 18, 2012 at 12:13:28PM -0800, Ian Romanick wrote: > On 01/18/2012 02:21 AM, Yuanhan Liu wrote: > >On Wed, Jan 18, 2012 at 06:23:52PM +0800, Yuanhan Liu wrote: > >>When rendering to FBO, rendering is inverted. At the same time, we would > >>also make sure the point sprite origin is inverted. Or, we will get an > >>inverted result correspoinding to rendering to the default winsys FBO. > >> > >>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613 > > > >Actually, I'm not sure about this patch, and I would like to treat this > >patch as a _workaround_. So, any comments are welcome. > > Why? Do you have a different solution in mind? Since I tried to 'fix' this issue. Acutally, this is the first "workaround fix" I thought when met this issue. I just don't know why then. Well, Nanhai's suggestion told me that this _might_ be the right fix for this issue. But I'm still a little unsure, so that's why. > This patch fixes > this problem the way I would have expected it to be fixed. Yes, it does. Thanks, Yuanhan Liu ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO
On Wed, Jan 18, 2012 at 11:53:20AM -0800, Ian Romanick wrote: > On 01/18/2012 02:23 AM, Yuanhan Liu wrote: > >When rendering to FBO, rendering is inverted. At the same time, we would > >also make sure the point sprite origin is inverted. Or, we will get an > >inverted result correspoinding to rendering to the default winsys FBO. > > > >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613 > > > >Signed-off-by: Yuanhan Liu > >--- > > src/mesa/drivers/dri/i965/brw_defines.h |1 + > > src/mesa/drivers/dri/i965/gen6_sf_state.c | 19 +-- > > 2 files changed, 18 insertions(+), 2 deletions(-) > > > >diff --git a/src/mesa/drivers/dri/i965/brw_defines.h > >b/src/mesa/drivers/dri/i965/brw_defines.h > >index 4d90a99..029be87 100644 > >--- a/src/mesa/drivers/dri/i965/brw_defines.h > >+++ b/src/mesa/drivers/dri/i965/brw_defines.h > >@@ -1128,6 +1128,7 @@ enum brw_message_target { > > /* DW1 (for gen6) */ > > # define GEN6_SF_NUM_OUTPUTS_SHIFT 22 > > # define GEN6_SF_SWIZZLE_ENABLE(1<< 21) > >+# define GEN6_SF_POINT_SPRITE_UPPERLEFT (0<< 20) > > # define GEN6_SF_POINT_SPRITE_LOWERLEFT(1<< 20) > > # define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT 11 > > # define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT 4 > >diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c > >b/src/mesa/drivers/dri/i965/gen6_sf_state.c > >index 548c5a3..d354a2b 100644 > >--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c > >+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c > > I think gen7_sf_state.c needs similar changes. Yeah, you are right. > > >@@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw) > > float point_size; > > uint16_t attr_overrides[FRAG_ATTRIB_MAX]; > > bool userclip_active; > >+ int point_sprite_origin; > > dw1 is a uint32_t, so this should be too. Yes. Will sent an updated patch soon. > > > > > /* _NEW_TRANSFORM */ > > userclip_active = (ctx->Transform.ClipPlanesEnabled != 0); > >@@ -258,8 +259,22 @@ upload_sf_state(struct brw_context *brw) > > /* Clamp to the hardware limits and convert to fixed point */ > > dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3); > > > >- if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT) > >- dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT; > >+ /* > >+* When rendering to FBO, rendering is inverted. At the same time, > >+* we would also make sure the point sprite origin is inverted. > >+* Or, we will get an inverted result corresponding to rendering > >+* to the default/window FBO. > >+*/ > >+ if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT) { > >+ point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT; > >+ if (render_to_fbo) > >+ point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT; > >+ } else { > >+ point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT; > >+ if (render_to_fbo) > >+ point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT; > >+ } > >+ dw1 |= point_sprite_origin; > > > > /* _NEW_LIGHT */ > > if (ctx->Light.ProvokingVertex != GL_FIRST_VERTEX_CONVENTION) { ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO
When rendering to FBO, rendering is inverted. At the same time, we would also make sure the point sprite origin is inverted. Or, we will get an inverted result correspoinding to rendering to the default winsys FBO. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613 NOTE: This is a candidate for stable release branches. v2: add the simliar logic to ivb, too (comments from Ian) simplify the logic operation (comments from Brian) Signed-off-by: Yuanhan Liu --- src/mesa/drivers/dri/i965/brw_defines.h |1 + src/mesa/drivers/dri/i965/gen6_sf_state.c | 15 +-- src/mesa/drivers/dri/i965/gen7_sf_state.c | 20 +--- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h index 4d90a99..029be87 100644 --- a/src/mesa/drivers/dri/i965/brw_defines.h +++ b/src/mesa/drivers/dri/i965/brw_defines.h @@ -1128,6 +1128,7 @@ enum brw_message_target { /* DW1 (for gen6) */ # define GEN6_SF_NUM_OUTPUTS_SHIFT 22 # define GEN6_SF_SWIZZLE_ENABLE(1 << 21) +# define GEN6_SF_POINT_SPRITE_UPPERLEFT(0 << 20) # define GEN6_SF_POINT_SPRITE_LOWERLEFT(1 << 20) # define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT 11 # define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT 4 diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c index 548c5a3..67c208b 100644 --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c @@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw) float point_size; uint16_t attr_overrides[FRAG_ATTRIB_MAX]; bool userclip_active; + uint32_t point_sprite_origin; /* _NEW_TRANSFORM */ userclip_active = (ctx->Transform.ClipPlanesEnabled != 0); @@ -258,8 +259,18 @@ upload_sf_state(struct brw_context *brw) /* Clamp to the hardware limits and convert to fixed point */ dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3); - if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT) - dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT; + /* +* When rendering to FBO, rendering is inverted. At the same time, +* we would also make sure the point sprite origin is inverted. +* Or, we will get an inverted result corresponding to rendering +* to the default/window FBO. +*/ + if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) ^ render_to_fbo) { + point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT; + } else { + point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT; + } + dw1 |= point_sprite_origin; /* _NEW_LIGHT */ if (ctx->Light.ProvokingVertex != GL_FIRST_VERTEX_CONVENTION) { diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c b/src/mesa/drivers/dri/i965/gen7_sf_state.c index 7691cb2..75a1cb0 100644 --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c @@ -48,6 +48,9 @@ upload_sbe_state(struct brw_context *brw) int urb_entry_read_offset = 1; bool userclip_active = (ctx->Transform.ClipPlanesEnabled != 0); uint16_t attr_overrides[FRAG_ATTRIB_MAX]; + /* _NEW_BUFFERS */ + bool render_to_fbo = brw->intel.ctx.DrawBuffer->Name != 0; + uint32_t point_sprite_origin; brw_compute_vue_map(&vue_map, intel, userclip_active, vs_outputs_written); urb_entry_read_length = (vue_map.num_slots + 1)/2 - urb_entry_read_offset; @@ -65,9 +68,20 @@ upload_sbe_state(struct brw_context *brw) urb_entry_read_length << GEN7_SBE_URB_ENTRY_READ_LENGTH_SHIFT | urb_entry_read_offset << GEN7_SBE_URB_ENTRY_READ_OFFSET_SHIFT; - /* _NEW_POINT */ - if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT) - dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT; + /* _NEW_POINT +* +* When rendering to FBO, rendering is inverted. At the same time, +* we would also make sure the point sprite origin is inverted. +* Or, we will get an inverted result corresponding to rendering +* to the default/window FBO. +*/ + if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) ^ render_to_fbo) { + point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT; + } else { + point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT; + } + dw1 |= point_sprite_origin; + dw10 = 0; dw11 = 0; -- 1.7.4.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/7] i965: use AM_CPPFLAGS instead of AM_CFLAGS/CXXFLAGS
On 12-01-17 08:28 PM, Matt Turner wrote: > On Tue, Jan 17, 2012 at 7:21 PM, Gaetan Nadon wrote: >> On 12-01-17 03:58 PM, Matt Turner wrote: >>> --- >>> src/mesa/drivers/dri/i965/Makefile.am |4 +--- >>> 1 files changed, 1 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/mesa/drivers/dri/i965/Makefile.am >>> b/src/mesa/drivers/dri/i965/Makefile.am >>> index 5512381..93937b1 100644 >>> --- a/src/mesa/drivers/dri/i965/Makefile.am >>> +++ b/src/mesa/drivers/dri/i965/Makefile.am >>> @@ -26,7 +26,7 @@ include Makefile.sources >>> # Hack to make some of the non-automake variables work. >>> TOP=$(top_builddir) >>> >>> -AM_CFLAGS = \ >>> +AM_CPPFLAGS = \ >>> -I$(top_srcdir)/include \ >>> -I$(top_srcdir)/src/ \ >>> -I$(top_srcdir)/src/mapi \ >> This looks suspicious and/or error prone. Are you sure that none of the >> variables assigned to AM_CPPFLAGS will ever contain any C compiler >> flags? While AM_CFLAGS can contain pre-processor flags, AM_CPPFLAGS >> cannot contain C compiler flags. I was not able to find where >> INTEL_CFLAGS is assigned, but as its name implies, it may contain C >> flags. If it is not used, it should be deleted. Automake has all the >> features to allow a user to append C flags on the make command line. > It's a little unclear, but I think > > PKG_CHECK_MODULES([INTEL], [libdrm_intel >= $LIBDRM_INTEL_REQUIRED]) > > is defining INTEL_{CFLAGS,LIBS}: > > INTEL_CFLAGS = -I/usr/include/libdrm > INTEL_LIBS = -ldrm_intel -ldrm I thought it was a private variable. Anyway, now I am sure there is issue. You cannot be sure that libdrm or any package for that matter will not put a compiler flag in there. In fact, that is what it was designed for. --cflags This prints pre-processor and compile flags required to compile the packages on the command line, including flags for all their dependencies. Flags are "compressed" so that each identical flag appears only once. pkg-config exits with a nonzero code if it can't find metadata for one or more of the packages on the com- mand line. For example, the xserver put the visibility flag in the xorg-server.pc file: Cflags: -I${sdkdir} -fvisibility=hidden There is this option which has never been used (which would have to be in pkg-config >= 0.22): --cflags-only-I This prints the -I part of "--cflags". That is, it defines the header search path but doesn't specify anything else. I guess it all depends on the level of risk one is willing to take. If you control libdrm and can make sure of what goes in there... > > and for radeon and nouveau: > > RADEON_CFLAGS = -I/usr/include/libdrm > RADEON_LIBS = -ldrm_radeon > > NOUVEAU_CFLAGS = -I/usr/include/libdrm -I/usr/include/nouveau > NOUVEAU_LIBS = -ldrm_nouveau > > so those look appropriate for AM_CPPFLAGS, right? > > Thanks, > Matt > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/7] automake: src/mesa/drivers/dri/nouveau
On 12-01-17 08:30 PM, Matt Turner wrote: > On Tue, Jan 17, 2012 at 7:32 PM, Gaetan Nadon wrote: >> On 12-01-17 03:58 PM, Matt Turner wrote: >> >> +AM_CPPFLAGS = \ >> +-I$(top_srcdir)/include \ >> +-I$(top_srcdir)/src/ \ >> +-I$(top_srcdir)/src/mapi \ >> +-I$(top_srcdir)/src/mesa/ \ >> +-I../common \ >> >> Why does this one cannot be written properly based on top_srcdir? > It can. I suppose not writing it in terms of top_srcdir would break > out-of-tree builds? Generally speaking, yes. There are cases where it will and cases where it won't as automake tries to correct dangerous situations. Using top_srcdir and top_builddir appropriately is always safe and easier to read than a bunch of ../../../ > > Thanks, > Matt > Thanks for listening! ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/7] i965: use AM_CPPFLAGS instead of AM_CFLAGS/CXXFLAGS
On 12-01-18 01:11 PM, Eric Anholt wrote: > On Tue, 17 Jan 2012 15:58:11 -0500, Matt Turner wrote: > > As far as I can tell, the point of AM_CPPFLAGS existing is if you want > to make a target that separately calls the preprocessor, so you a list > of includes and a list of non-preprocessor compiler flags. Given that > we don't have any preprocessor invocations, we're sure to leak actual > cflags into the cppflags, so I don't see why we'd make this change.. That's the only scenario I am aware of that will actually "break". You never know when this target might be added in the makefile. It's preferable to keep the flags separate where possible. Automake keeps them separate from top to bottom. A builder can append flags at make invocation using CFLAGS and CPPFLAGS, knowing they will used with the correct target. Unfortunately too many developers don't bother and when mixed flags are passed in through a variable from another module, they cannot be separated again. In another post, I expressed my reservations due to INTEL_CFLAGS that may contain mixed flags. Using AM_CFLAGS would be safer, unfortunately. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev