Re: [Mesa-dev] [PATCH 1/5] i965/fs: Set force_writemask_all on shader_time instructions.
On Sun, Mar 8, 2015 at 1:08 AM, Kenneth Graunke kenn...@whitecape.org wrote: These computations don't have anything to do with the currently executing channels, so they should use force_writemask_all. This fixes assert failures. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86974 Signed-off-by: Kenneth Graunke kenn...@whitecape.org Cc: mesa-sta...@lists.freedesktop.org --- src/mesa/drivers/dri/i965/brw_fs.cpp | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index d6acc23..45a5793 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -759,18 +759,23 @@ fs_visitor::emit_shader_time_end() reset.set_smear(2); fs_inst *test = emit(AND(reg_null_d, reset, fs_reg(1u))); test-conditional_mod = BRW_CONDITIONAL_Z; + test-force_writemask_all = true; emit(IF(BRW_PREDICATE_NORMAL)); fs_reg start = shader_start_time; start.negate = true; fs_reg diff = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD, 1); - emit(ADD(diff, start, shader_end_time)); + fs_inst *add = ADD(diff, start, shader_end_time); + add-force_writemask_all = true; + emit(add); Emit returns a pointer to the instruction it emitted, so you can cut one line by wrapping the ADD with emit(). Either way's fine. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
On Sat, Mar 07, 2015 at 04:15:08PM +0200, Francisco Jerez wrote: Topi Pohjolainen topi.pohjolai...@intel.com writes: The original patch from Curro was based on something that is not present in the master yet. This patch tries to mimick the logic on top master. I wanted to see if could separate the building of the descriptor instruction from building of the send instruction. This logic now allows the caller to construct any kind of sequence of instructions filling in the descriptor before giving it to the send instruction builder. This is only compile tested. Curro, how would you feel about this sort of approach? I apologise for muddying the waters again but I wasn't entirely comfortable with the logic and wanted to try to something else. I believe patch number five should go nicely on top of this as the descriptor instruction could be followed by (or preceeeded by) any additional instructions modifying the descriptor register before the actual send instruction. Topi, the goal I had in mind with PATCH 01 was to refactor a commonly recurring pattern. In terms of the functions defined in this patch my example from yesterday [1] would now look like: | if (index.file == BRW_IMMEDIATE_VALUE) { | | uint32_t surf_index = index.dw1.ud; | | brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND); | brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); | brw_set_src0(p, send, offset); | brw_set_sampler_message(p, send, | surf_index, | 0, /* LD message ignores sampler unit */ | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen, | mlen, | false, /* no header */ | simd_mode, | 0); | | brw_mark_surface_used(prog_data, surf_index); | | } else { | | struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); | | brw_push_insn_state(p); | brw_set_default_mask_control(p, BRW_MASK_DISABLE); | brw_set_default_access_mode(p, BRW_ALIGN_1); | | /* a0.0 = surf_index 0xff */ | brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); | brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); | brw_set_dest(p, insn_and, addr); | brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); | brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); | | | /* a0.0 |= descriptor */ | brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); | brw_set_sampler_message(p, descr_inst, | 0 /* surface */, | 0 /* sampler */, | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen /* rlen */, | mlen /* mlen */, | false /* header */, | simd_mode, | 0); | | /* dst = send(offset, a0.0) */ | brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); | | brw_pop_insn_state(p); | | /* visitor knows more than we do about the surface limit required, | * so has already done marking. | */ | } Which I think could also be written as follows. Or am I missing something again? static brw_inst * brw_build_surface_index_descr(struct brw_compile *p, struct brw_reg dst, index) { brw_set_default_mask_control(p, BRW_MASK_DISABLE); brw_set_default_access_mode(p, BRW_ALIGN_1); /* a0.0 = surf_index 0xff */ brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); brw_set_dest(p, insn_and, addr); brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); /* a0.0 |= descriptor */ brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); } ... brw_inst *descr_inst; if (index.file == BRW_IMMEDIATE_VALUE) { descr = brw_next_insn(p, BRW_OPCODE_SEND); brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); brw_set_src0(p, send, offset); brw_mark_surface_used(prog_data, surf_index); } else { struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); brw_push_insn_state(p); brw_build_surface_index_descr(p, addr, index); /* dst = send(offset, a0.0) */ descr_inst = brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); brw_pop_insn_state(p); } uint32_t surf_index = index.file == BRW_IMMEDIATE_VALUE ? index.dw1.ud : 0; brw_set_sampler_message(p, descr_inst, surf_index,
Re: [Mesa-dev] [PATCH 1/5] i965/fs: Set force_writemask_all on shader_time instructions.
On Sunday, March 08, 2015 01:32:33 PM Matt Turner wrote: On Sun, Mar 8, 2015 at 1:08 AM, Kenneth Graunke kenn...@whitecape.org wrote: These computations don't have anything to do with the currently executing channels, so they should use force_writemask_all. This fixes assert failures. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86974 Signed-off-by: Kenneth Graunke kenn...@whitecape.org Cc: mesa-sta...@lists.freedesktop.org --- src/mesa/drivers/dri/i965/brw_fs.cpp | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index d6acc23..45a5793 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -759,18 +759,23 @@ fs_visitor::emit_shader_time_end() reset.set_smear(2); fs_inst *test = emit(AND(reg_null_d, reset, fs_reg(1u))); test-conditional_mod = BRW_CONDITIONAL_Z; + test-force_writemask_all = true; emit(IF(BRW_PREDICATE_NORMAL)); fs_reg start = shader_start_time; start.negate = true; fs_reg diff = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD, 1); - emit(ADD(diff, start, shader_end_time)); + fs_inst *add = ADD(diff, start, shader_end_time); + add-force_writemask_all = true; + emit(add); Emit returns a pointer to the instruction it emitted, so you can cut one line by wrapping the ADD with emit(). Either way's fine. Yeah, I didn't bother because patch 5 changes it to: end-insert_before(add) and that doesn't return a pointer to the inserted instruction. 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] nvc0: fix wrong max value for driver queries
On 03/08/2015 05:45 PM, Ilia Mirkin wrote: On Sun, Mar 8, 2015 at 12:18 PM, Samuel Pitoiset samuel.pitoi...@gmail.com wrote: The maximum value of a Gallium HUD's panel is automatically adjusted when the current value is greater than the max. If we set the pipe_query_driver_info::max_value to UINT64_MAX, the maximum value is never adjusted and this results in a right instead of a pretty curve I'm guessing right is translated from droite? In english, that's straight line. Yes, thanks. Can you confirm that this is what other drivers do? Another interpretation of max_value is max possible value, and the HUD should keep track of its own max seen thing. Yes, other drivers set max_value to 0 in order to keep track of this max seen value as you said. which is correctly scaled. Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com --- src/gallium/drivers/nouveau/nvc0/nvc0_query.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c index ec464b5..f21deea 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c @@ -1418,7 +1418,7 @@ nvc0_screen_get_driver_query_info(struct pipe_screen *pscreen, if (id NVC0_QUERY_DRV_STAT_COUNT) { info-name = nvc0_drv_stat_names[id]; info-query_type = NVC0_QUERY_DRV_STAT(id); - info-max_value = ~0ULL; + info-max_value = 0; info-uses_byte_units = !!strstr(info-name, bytes); return 1; } else @@ -1427,15 +1427,14 @@ nvc0_screen_get_driver_query_info(struct pipe_screen *pscreen, if (screen-base.class_3d = NVE4_3D_CLASS) { info-name = nve4_pm_query_names[id - NVC0_QUERY_DRV_STAT_COUNT]; info-query_type = NVE4_PM_QUERY(id - NVC0_QUERY_DRV_STAT_COUNT); - info-max_value = (id NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) ? -~0ULL : 100; + info-max_value = (id NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) ? 0 : 100; info-uses_byte_units = FALSE; return 1; } else if (screen-compute) { info-name = nvc0_pm_query_names[id - NVC0_QUERY_DRV_STAT_COUNT]; info-query_type = NVC0_PM_QUERY(id - NVC0_QUERY_DRV_STAT_COUNT); - info-max_value = ~0ULL; + info-max_value = 0; info-uses_byte_units = FALSE; return 1; } -- 2.3.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 2/4] i965/fs: Make get_timestamp() return an fs_inst * rather than emitting.
On Friday, February 27, 2015 11:37:01 PM Pohjolainen, Topi wrote: On Fri, Feb 27, 2015 at 11:15:35AM -0800, Kenneth Graunke wrote: This makes another part of the INTEL_DEBUG=shader_time code emittable at arbitrary locations, rather than just at the end of the instruction stream. v2: Don't lose smear! Caught by Topi Pohjolainen. Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/drivers/dri/i965/brw_fs.cpp | 24 +--- src/mesa/drivers/dri/i965/brw_fs.h | 2 +- 2 files changed, 14 insertions(+), 12 deletions(-) Yikes, good catch! Thanks for the review, Topi! diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 9c6f084..d65f1f1 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -680,8 +680,8 @@ fs_visitor::type_size(const struct glsl_type *type) return 0; } -fs_reg -fs_visitor::get_timestamp() +fs_inst * +fs_visitor::timestamp_read() { assert(brw-gen = 7); @@ -692,12 +692,6 @@ fs_visitor::get_timestamp() fs_reg dst = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD, 4); - fs_inst *mov = emit(MOV(dst, ts)); - /* We want to read the 3 fields we care about even if it's not enabled in -* the dispatch. -*/ - mov-force_writemask_all = true; - /* The caller wants the low 32 bits of the timestamp. Since it's running * at the GPU clock rate of ~1.2ghz, it will roll over every ~3 seconds, * which is plenty of time for our purposes. It is identical across the @@ -710,14 +704,21 @@ fs_visitor::get_timestamp() */ dst.set_smear(0); - return dst; + fs_inst *mov = MOV(dst, ts); Previously the smear wasn't set for the destination in the instruction itself. I had to check what set_smear() really does. It also sets stride to zero which the original logic left to the init value of one. I guess this is not what you intented? Augh. Good catch - that totally breaks things. It makes the timestamp read a mov(1) instead of a mov(4). I rebased this code, and discovered that shader_time was just totally hosed again. I think I've patched it up, and will send out a respin shortly. 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
[Mesa-dev] [PATCH 5/5] i965/fs: Make emit_shader_time_end() insert before EOT.
Previously, we emitted the shader-time epilogue from emit_fb_writes(), during the middle of looping through color regions (or emit_urb_writes for the VS). This is duplicated several times and rather awkward. I need to fix a bug in our FB write handling, and it will be a lot easier if we move emit_shader_time_end() out of there. Now, we simply emit FB writes/URB writes, and subsequently have emit_shader_time_end() insert instructions before the final SEND with EOT. Not only is this simpler, it's actually a slight improvement: we now include the MOVs to set up the final FB write payload in our shader-time measurements. Note that INTEL_DEBUG=shader_time only exists on Gen7+, and uses send-from-GRF. (In the past, we might have hit trouble where both attempt to use MRFs for messages; that's not a problem now.) v2: Rebase on v3 of the previous patch and other shader_time fixes. Signed-off-by: Kenneth Graunke kenn...@whitecape.org Reviewed-by: Topi Pohjolainen topi.pohjolai...@intel.com [v1] --- src/mesa/drivers/dri/i965/brw_fs.cpp | 28 ++-- src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 13 - 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 8f11600..af48f4b 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -759,19 +759,24 @@ fs_visitor::emit_shader_time_end() unreachable(fs_visitor::emit_shader_time_end missing code); } + /* Insert our code just before the final SEND with EOT. */ + exec_node *end = this-instructions.get_tail(); + assert(end ((fs_inst *) end)-eot); + fs_inst *tm_read; fs_reg shader_end_time = get_timestamp(tm_read); - emit(tm_read); + end-insert_before(tm_read); /* Check that there weren't any timestamp reset events (assuming these * were the only two timestamp reads that happened). */ fs_reg reset = shader_end_time; reset.set_smear(2); - fs_inst *test = emit(AND(reg_null_d, reset, fs_reg(1u))); + fs_inst *test = AND(reg_null_d, reset, fs_reg(1u)); test-conditional_mod = BRW_CONDITIONAL_Z; test-force_writemask_all = true; - emit(IF(BRW_PREDICATE_NORMAL)); + end-insert_before(test); + end-insert_before(IF(BRW_PREDICATE_NORMAL)); fs_reg start = shader_start_time; start.negate = true; @@ -779,7 +784,7 @@ fs_visitor::emit_shader_time_end() diff.set_smear(0); fs_inst *add = ADD(diff, start, shader_end_time); add-force_writemask_all = true; - emit(add); + end-insert_before(add); /* If there were no instructions between the two timestamp gets, the diff * is 2 cycles. Remove that overhead, so I can forget about that when @@ -787,13 +792,13 @@ fs_visitor::emit_shader_time_end() */ add = ADD(diff, diff, fs_reg(-2u)); add-force_writemask_all = true; - emit(add); + end-insert_before(add); - emit(SHADER_TIME_ADD(type, diff)); - emit(SHADER_TIME_ADD(written_type, fs_reg(1u))); - emit(BRW_OPCODE_ELSE); - emit(SHADER_TIME_ADD(reset_type, fs_reg(1u))); - emit(BRW_OPCODE_ENDIF); + end-insert_before(SHADER_TIME_ADD(type, diff)); + end-insert_before(SHADER_TIME_ADD(written_type, fs_reg(1u))); + end-insert_before(new(mem_ctx) fs_inst(BRW_OPCODE_ELSE, dispatch_width)); + end-insert_before(SHADER_TIME_ADD(reset_type, fs_reg(1u))); + end-insert_before(new(mem_ctx) fs_inst(BRW_OPCODE_ENDIF, dispatch_width)); } fs_inst * @@ -3915,6 +3920,9 @@ fs_visitor::run_fs() emit_fb_writes(); + if (INTEL_DEBUG DEBUG_SHADER_TIME) + emit_shader_time_end(); + calculate_cfg(); optimize(); diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp index 6b48f70..6766583 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp @@ -3645,9 +3645,6 @@ fs_visitor::emit_fb_writes() fs_inst *inst; if (do_dual_src) { - if (INTEL_DEBUG DEBUG_SHADER_TIME) - emit_shader_time_end(); - this-current_annotation = ralloc_asprintf(this-mem_ctx, FB dual-source write); inst = emit_single_fb_write(this-outputs[0], this-dual_src_output, @@ -3663,19 +3660,12 @@ fs_visitor::emit_fb_writes() if (brw-gen = 6 key-replicate_alpha target != 0) src0_alpha = offset(outputs[0], 3); - if (target == key-nr_color_regions - 1 - (INTEL_DEBUG DEBUG_SHADER_TIME)) -emit_shader_time_end(); - inst = emit_single_fb_write(this-outputs[target], reg_undef, src0_alpha, this-output_components[target]); inst-target = target; } } else { - if (INTEL_DEBUG DEBUG_SHADER_TIME) - emit_shader_time_end(); - /* Even if there's no color
[Mesa-dev] [PATCH 2/5] i965/fs: Set smear on shader_time diff register.
The ADD(diff, diff, fs_reg(-2u)) instruction reads diff, which is a width 1 register. We need to read it as 0,1,0 with a subreg of 0, which is what smear accomplishes. Fixes assertion: brw_eu_emit.c:285: validate_reg: Assertion `hstride == 0' failed. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86974 Signed-off-by: Kenneth Graunke kenn...@whitecape.org Cc: mesa-sta...@lists.freedesktop.org --- src/mesa/drivers/dri/i965/brw_fs.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 45a5793..db8affa 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -765,6 +765,7 @@ fs_visitor::emit_shader_time_end() fs_reg start = shader_start_time; start.negate = true; fs_reg diff = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD, 1); + diff.set_smear(0); fs_inst *add = ADD(diff, start, shader_end_time); add-force_writemask_all = true; emit(add); -- 2.2.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/5] i965/fs: Set force_writemask_all on shader_time instructions.
These computations don't have anything to do with the currently executing channels, so they should use force_writemask_all. This fixes assert failures. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86974 Signed-off-by: Kenneth Graunke kenn...@whitecape.org Cc: mesa-sta...@lists.freedesktop.org --- src/mesa/drivers/dri/i965/brw_fs.cpp | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index d6acc23..45a5793 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -759,18 +759,23 @@ fs_visitor::emit_shader_time_end() reset.set_smear(2); fs_inst *test = emit(AND(reg_null_d, reset, fs_reg(1u))); test-conditional_mod = BRW_CONDITIONAL_Z; + test-force_writemask_all = true; emit(IF(BRW_PREDICATE_NORMAL)); fs_reg start = shader_start_time; start.negate = true; fs_reg diff = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD, 1); - emit(ADD(diff, start, shader_end_time)); + fs_inst *add = ADD(diff, start, shader_end_time); + add-force_writemask_all = true; + emit(add); /* If there were no instructions between the two timestamp gets, the diff * is 2 cycles. Remove that overhead, so I can forget about that when * trying to determine the time taken for single instructions. */ - emit(ADD(diff, diff, fs_reg(-2u))); + add = ADD(diff, diff, fs_reg(-2u)); + add-force_writemask_all = true; + emit(add); emit_shader_time_write(type, diff); emit_shader_time_write(written_type, fs_reg(1u)); -- 2.2.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/5] i965/fs: Make emit_shader_time_write return rather than emit.
Instead of emit_shader_time_write, we now do emit(SHADER_TIME_ADD(...)). The advantage is that we can also insert a shader time write at an arbitrary location in the instruction stream, rather than being restricted to emitting at the end. Signed-off-by: Kenneth Graunke kenn...@whitecape.org Reviewed-by: Topi Pohjolainen topi.pohjolai...@intel.com --- src/mesa/drivers/dri/i965/brw_fs.cpp | 15 +++ src/mesa/drivers/dri/i965/brw_fs.h | 3 +-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index db8affa..682841b 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -778,16 +778,15 @@ fs_visitor::emit_shader_time_end() add-force_writemask_all = true; emit(add); - emit_shader_time_write(type, diff); - emit_shader_time_write(written_type, fs_reg(1u)); + emit(SHADER_TIME_ADD(type, diff)); + emit(SHADER_TIME_ADD(written_type, fs_reg(1u))); emit(BRW_OPCODE_ELSE); - emit_shader_time_write(reset_type, fs_reg(1u)); + emit(SHADER_TIME_ADD(reset_type, fs_reg(1u))); emit(BRW_OPCODE_ENDIF); } -void -fs_visitor::emit_shader_time_write(enum shader_time_shader_type type, - fs_reg value) +fs_inst * +fs_visitor::SHADER_TIME_ADD(enum shader_time_shader_type type, fs_reg value) { int shader_time_index = brw_get_shader_time_index(brw, shader_prog, prog, type); @@ -799,8 +798,8 @@ fs_visitor::emit_shader_time_write(enum shader_time_shader_type type, else payload = vgrf(glsl_type::uint_type); - emit(new(mem_ctx) fs_inst(SHADER_OPCODE_SHADER_TIME_ADD, - fs_reg(), payload, offset, value)); + return new(mem_ctx) fs_inst(SHADER_OPCODE_SHADER_TIME_ADD, + fs_reg(), payload, offset, value); } void diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h index 70098d8..be1c8a1 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.h +++ b/src/mesa/drivers/dri/i965/brw_fs.h @@ -378,8 +378,7 @@ public: void emit_shader_time_begin(); void emit_shader_time_end(); - void emit_shader_time_write(enum shader_time_shader_type type, - fs_reg value); + fs_inst *SHADER_TIME_ADD(enum shader_time_shader_type type, fs_reg value); void emit_untyped_atomic(unsigned atomic_op, unsigned surf_index, fs_reg dst, fs_reg offset, fs_reg src0, -- 2.2.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/5] i965/fs: Make get_timestamp() pass back the MOV rather than emitting it.
This makes another part of the INTEL_DEBUG=shader_time code emittable at arbitrary locations, rather than just at the end of the instruction stream. v2: Don't lose smear! Caught by Topi Pohjolainen. v3: Don't set smear on the destination of the MOV. Thanks Topi! Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/drivers/dri/i965/brw_fs.cpp | 19 +++ src/mesa/drivers/dri/i965/brw_fs.h | 2 +- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 682841b..8f11600 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -677,8 +677,14 @@ fs_visitor::type_size(const struct glsl_type *type) return 0; } +/** + * Create a MOV to read the timestamp register. + * + * The caller is responsible for emitting the MOV. The return value is + * the destination of the MOV, with extra parameters set. + */ fs_reg -fs_visitor::get_timestamp() +fs_visitor::get_timestamp(fs_inst **out_mov) { assert(brw-gen = 7); @@ -689,7 +695,7 @@ fs_visitor::get_timestamp() fs_reg dst = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD, 4); - fs_inst *mov = emit(MOV(dst, ts)); + fs_inst *mov = MOV(dst, ts); /* We want to read the 3 fields we care about even if it's not enabled in * the dispatch. */ @@ -707,6 +713,7 @@ fs_visitor::get_timestamp() */ dst.set_smear(0); + *out_mov = mov; return dst; } @@ -714,7 +721,9 @@ void fs_visitor::emit_shader_time_begin() { current_annotation = shader time start; - shader_start_time = get_timestamp(); + fs_inst *mov; + shader_start_time = get_timestamp(mov); + emit(mov); } void @@ -750,7 +759,9 @@ fs_visitor::emit_shader_time_end() unreachable(fs_visitor::emit_shader_time_end missing code); } - fs_reg shader_end_time = get_timestamp(); + fs_inst *tm_read; + fs_reg shader_end_time = get_timestamp(tm_read); + emit(tm_read); /* Check that there weren't any timestamp reset events (assuming these * were the only two timestamp reads that happened). diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h index be1c8a1..f68a476 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.h +++ b/src/mesa/drivers/dri/i965/brw_fs.h @@ -399,7 +399,7 @@ public: void resolve_ud_negate(fs_reg *reg); void resolve_bool_comparison(ir_rvalue *rvalue, fs_reg *reg); - fs_reg get_timestamp(); + fs_reg get_timestamp(fs_inst **out_mov); struct brw_reg interp_reg(int location, int channel); void setup_uniform_values(ir_variable *ir); -- 2.2.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] INTEL_DEBUG=shader_time scalar backend fixes
Welcome to the rabbit trail. In order to fix Football Manager, I had to rework INTEL_DEBUG=shader_time in the FS backend. While doing that, I hit two assertion failures. After fixing that, I compared numbers. I noticed that VS again accounted for 0 cycles. Matt - could you take a look at brw_vec4_dead_code_elimination.cpp? Since 5df88c2096281f (the rewrite to use live intervals), it's again completely eating the atomic buffer offset initialization, resulting in us using garbage data in the first register (of an mlen 2 message). (Sounds like the same bug I fixed in d0575d98fc595dcc17706dc73d1eb4.) With these patches, and the new vec4 DCE pass reverted, 10.3 vs my branch produce basically the same numbers on an openarena timedemo. Available in the 'football-v3' branch of my tree. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: drop unused texture function
Reviewed-by: Ilia Mirkin imir...@alum.mit.edu On Sun, Mar 8, 2015 at 7:52 PM, Dave Airlie airl...@gmail.com wrote: This has no users. Signed-off-by: Dave Airlie airl...@redhat.com --- src/mesa/state_tracker/st_texture.c | 40 - src/mesa/state_tracker/st_texture.h | 10 -- 2 files changed, 50 deletions(-) diff --git a/src/mesa/state_tracker/st_texture.c b/src/mesa/state_tracker/st_texture.c index ada9841..ca7c83c 100644 --- a/src/mesa/state_tracker/st_texture.c +++ b/src/mesa/state_tracker/st_texture.c @@ -310,46 +310,6 @@ st_texture_image_unmap(struct st_context *st, *transfer = NULL; } - -/* Upload data for a particular image. - */ -void -st_texture_image_data(struct st_context *st, - struct pipe_resource *dst, - GLuint face, - GLuint level, - void *src, - GLuint src_row_stride, GLuint src_image_stride) -{ - struct pipe_context *pipe = st-pipe; - GLuint i; - const GLubyte *srcUB = src; - GLuint layers; - - if (dst-target == PIPE_TEXTURE_1D_ARRAY || - dst-target == PIPE_TEXTURE_2D_ARRAY || - dst-target == PIPE_TEXTURE_CUBE_ARRAY) - layers = dst-array_size; - else - layers = u_minify(dst-depth0, level); - - DBG(%s\n, __FUNCTION__); - - for (i = 0; i layers; i++) { - struct pipe_box box; - u_box_2d_zslice(0, 0, face + i, - u_minify(dst-width0, level), - u_minify(dst-height0, level), - box); - - pipe-transfer_inline_write(pipe, dst, level, PIPE_TRANSFER_WRITE, - box, srcUB, src_row_stride, 0); - - srcUB += src_image_stride; - } -} - - /** * For debug only: get/print center pixel in the src resource. */ diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h index 3fa55ae..d8cd7c7 100644 --- a/src/mesa/state_tracker/st_texture.h +++ b/src/mesa/state_tracker/st_texture.h @@ -224,16 +224,6 @@ st_texture_image_unmap(struct st_context *st, extern const GLuint * st_texture_depth_offsets(struct pipe_resource *pt, GLuint level); - -/* Upload an image into a texture - */ -extern void -st_texture_image_data(struct st_context *st, - struct pipe_resource *dst, - GLuint face, GLuint level, void *src, - GLuint src_row_pitch, GLuint src_image_pitch); - - /* Copy an image between two textures */ extern void -- 2.1.0 ___ 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] nesa-10.4.4: gallivm/lp_bld_misc.cpp:503:38: error: no viable conversion from 'ShaderMemoryManager *' to 'std::unique_ptrRTDyldMemoryManager'
On Fri, Mar 6, 2015 at 8:06 PM, Emil Velikov emil.l.veli...@gmail.com wrote: On 4 March 2015 at 18:07, Roland Scheidegger srol...@vmware.com wrote: Am 04.03.2015 um 12:38 schrieb Jose Fonseca: On 04/03/15 02:00, Emil Velikov wrote: On 27 February 2015 at 23:28, Sedat Dilek sedat.di...@gmail.com wrote: On Mon, Feb 9, 2015 at 6:30 PM, Emil Velikov emil.l.veli...@gmail.com wrote: On 07/02/15 21:44, Sedat Dilek wrote: Hi, I was building mesa v10.4.4 with my llvm-toolchain v3.6.0rc2. My build breaks like this... ... Please cherry-pick... commit ef7e0b39a24966526b102643523feac765771842 gallivm: Update for RTDyldMemoryManager becoming an unique_ptr. ..for mesa 10.4 Git branch. Hi Sedat, Picking a fix in a stable branch against a non-final release sounds like a no-go in our books. As the official llvm 3.6 rolls out we'll pick this fix for the stable branches - until then I would recommend (a) applying it locally or (b) using mesa from the 10.5 or master branch. Just FYI... [LLVMdev] LLVM 3.6 Release (see [1]). Please pick this patch for-10.4, thanks. As promised, mesa 10.4.6 will feature this. But is cross-porting this patch enough? As I said when this first issue was raised fixing the build with LLVM 3.6 is just half of the problem. It must also _run_ correctly. And building correctly doesn't necessarily means it will run correctly. That is, unless somebody actually ensures that all LLVM 3.6 related fixes have been crossported and that things run correctly, it is misleading to enable the build of Mesa 10.4.6 with LLVM 3.6. I don't know about radeon drivers, but at least from llvmpipe POV I simply don't have the time to do this (go through every LLVM 3.6 related patch, ensure they are all in 10.4.6, and test). I quickly went through the diffs between 10.4 branch, and found one such commit is missing: http://cgit.freedesktop.org/mesa/mesa/commit/?id=74f505fa73eda0c9b5b1984bebb44cedac8e8794 https://bugs.freedesktop.org/show_bug.cgi?id=85467 But there might be more, and I don't know if crossporting this is safe or not. Therefore my stance for is that building Mesa stable releases with LLVM releases after the Mesa release was branched is still unsupported. If people want to do so, they will do at their own peril. And any incoming bugs will be unsupported, use Mesa. If having a Mesa release capable of building LLVM 3.6 is so important, I think it might be easier/safer to just make a new release from a recent enough commit, than trying to backport it. This is quite right, the above commit is a must if you want to build with llvm 3.6. I am quite sure crossport should be safe (it missed the branch point of 10.4 just narrowly), and I don't think there's any other patches missing, but no guarantees... I think it is sort of unfortunate that the latest mesa release wouldn't run with the latest llvm release, but the fact remains that without testing this sounds all a bit risky... Thanks for the input gents. So the input so far we've got is that no-one is testing llvm 3.6 with mesa 10.4. I love to give it a spin, yet Archlinux doesn't have llvm 3.6 . There is also the double-free bug mentioned in https://bugs.freedesktop.org/show_bug.cgi?id=89387 All that said, Sedat I will revert the commit and release 10.4.6 without it. On the positive side, mesa 10.5.0 is coming out later on today, which should work like a charm with llvm 3.6. No, it breaks here as the (compiler) visibility fixes are still missing when building with --enable-glx-tls. - Sedat - ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/mesa: drop unused texture function
This has no users. Signed-off-by: Dave Airlie airl...@redhat.com --- src/mesa/state_tracker/st_texture.c | 40 - src/mesa/state_tracker/st_texture.h | 10 -- 2 files changed, 50 deletions(-) diff --git a/src/mesa/state_tracker/st_texture.c b/src/mesa/state_tracker/st_texture.c index ada9841..ca7c83c 100644 --- a/src/mesa/state_tracker/st_texture.c +++ b/src/mesa/state_tracker/st_texture.c @@ -310,46 +310,6 @@ st_texture_image_unmap(struct st_context *st, *transfer = NULL; } - -/* Upload data for a particular image. - */ -void -st_texture_image_data(struct st_context *st, - struct pipe_resource *dst, - GLuint face, - GLuint level, - void *src, - GLuint src_row_stride, GLuint src_image_stride) -{ - struct pipe_context *pipe = st-pipe; - GLuint i; - const GLubyte *srcUB = src; - GLuint layers; - - if (dst-target == PIPE_TEXTURE_1D_ARRAY || - dst-target == PIPE_TEXTURE_2D_ARRAY || - dst-target == PIPE_TEXTURE_CUBE_ARRAY) - layers = dst-array_size; - else - layers = u_minify(dst-depth0, level); - - DBG(%s\n, __FUNCTION__); - - for (i = 0; i layers; i++) { - struct pipe_box box; - u_box_2d_zslice(0, 0, face + i, - u_minify(dst-width0, level), - u_minify(dst-height0, level), - box); - - pipe-transfer_inline_write(pipe, dst, level, PIPE_TRANSFER_WRITE, - box, srcUB, src_row_stride, 0); - - srcUB += src_image_stride; - } -} - - /** * For debug only: get/print center pixel in the src resource. */ diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h index 3fa55ae..d8cd7c7 100644 --- a/src/mesa/state_tracker/st_texture.h +++ b/src/mesa/state_tracker/st_texture.h @@ -224,16 +224,6 @@ st_texture_image_unmap(struct st_context *st, extern const GLuint * st_texture_depth_offsets(struct pipe_resource *pt, GLuint level); - -/* Upload an image into a texture - */ -extern void -st_texture_image_data(struct st_context *st, - struct pipe_resource *dst, - GLuint face, GLuint level, void *src, - GLuint src_row_pitch, GLuint src_image_pitch); - - /* Copy an image between two textures */ extern void -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/4] Introduce get_device_vendor() entrypoint for pipes
On 07.03.2015 16:56, Giuseppe Bilotta wrote: This will be needed by Clover to return the correct information to CL_DEVICE_VENDOR info queries. --- src/gallium/docs/source/screen.rst | 6 ++ src/gallium/include/pipe/p_screen.h | 8 2 files changed, 14 insertions(+) diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst index e0fd1a2..6c717f4 100644 --- a/src/gallium/docs/source/screen.rst +++ b/src/gallium/docs/source/screen.rst @@ -460,6 +460,12 @@ get_vendor Returns the screen vendor. +get_device_vendor +^^ Extend this line to the length of the string. With that fixed and the shortlog prefixed by gallium:, Reviewed-by: Michel Dänzer michel.daen...@amd.com -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] Whitespace cleanup
On 07.03.2015 16:56, Giuseppe Bilotta wrote: --- src/gallium/include/pipe/p_screen.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h index ac88506..4018f8a 100644 --- a/src/gallium/include/pipe/p_screen.h +++ b/src/gallium/include/pipe/p_screen.h @@ -147,7 +147,7 @@ struct pipe_screen { */ boolean (*can_create_resource)(struct pipe_screen *screen, const struct pipe_resource *templat); - + /** * Create a new texture object, using the given template info. */ The shortlog should be something like gallium: Remove trailing whitespace from p_screen.h instead of just Whitespace cleanup. With that fixed, Reviewed-by: Michel Dänzer michel.daen...@amd.com The shortlog of patch 4 should be prefixed by gallium: as well. With that fixed, it's Acked-by: Michel Dänzer michel.daen...@amd.com -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nvc0: fix wrong max value for driver queries
The maximum value of a Gallium HUD's panel is automatically adjusted when the current value is greater than the max. If we set the pipe_query_driver_info::max_value to UINT64_MAX, the maximum value is never adjusted and this results in a right instead of a pretty curve which is correctly scaled. Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com --- src/gallium/drivers/nouveau/nvc0/nvc0_query.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c index ec464b5..f21deea 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c @@ -1418,7 +1418,7 @@ nvc0_screen_get_driver_query_info(struct pipe_screen *pscreen, if (id NVC0_QUERY_DRV_STAT_COUNT) { info-name = nvc0_drv_stat_names[id]; info-query_type = NVC0_QUERY_DRV_STAT(id); - info-max_value = ~0ULL; + info-max_value = 0; info-uses_byte_units = !!strstr(info-name, bytes); return 1; } else @@ -1427,15 +1427,14 @@ nvc0_screen_get_driver_query_info(struct pipe_screen *pscreen, if (screen-base.class_3d = NVE4_3D_CLASS) { info-name = nve4_pm_query_names[id - NVC0_QUERY_DRV_STAT_COUNT]; info-query_type = NVE4_PM_QUERY(id - NVC0_QUERY_DRV_STAT_COUNT); - info-max_value = (id NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) ? -~0ULL : 100; + info-max_value = (id NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) ? 0 : 100; info-uses_byte_units = FALSE; return 1; } else if (screen-compute) { info-name = nvc0_pm_query_names[id - NVC0_QUERY_DRV_STAT_COUNT]; info-query_type = NVC0_PM_QUERY(id - NVC0_QUERY_DRV_STAT_COUNT); - info-max_value = ~0ULL; + info-max_value = 0; info-uses_byte_units = FALSE; return 1; } -- 2.3.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4] mesa: improve ARB_copy_image internal format compat check
On Mar 7, 2015 10:33 PM, Kenneth Graunke kenn...@whitecape.org wrote: On Saturday, March 07, 2015 10:09:15 PM Jason Ekstrand wrote: LGTM. Jason, Sean doesn't have commit access (it's actually his first patch) - if this is good to go, would you mind pushing it for him? I thought as much. I'll push on Monday. --Jason Nice work, Sean! --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0: fix wrong max value for driver queries
On Sun, Mar 8, 2015 at 12:18 PM, Samuel Pitoiset samuel.pitoi...@gmail.com wrote: The maximum value of a Gallium HUD's panel is automatically adjusted when the current value is greater than the max. If we set the pipe_query_driver_info::max_value to UINT64_MAX, the maximum value is never adjusted and this results in a right instead of a pretty curve I'm guessing right is translated from droite? In english, that's straight line. Can you confirm that this is what other drivers do? Another interpretation of max_value is max possible value, and the HUD should keep track of its own max seen thing. which is correctly scaled. Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com --- src/gallium/drivers/nouveau/nvc0/nvc0_query.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c index ec464b5..f21deea 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c @@ -1418,7 +1418,7 @@ nvc0_screen_get_driver_query_info(struct pipe_screen *pscreen, if (id NVC0_QUERY_DRV_STAT_COUNT) { info-name = nvc0_drv_stat_names[id]; info-query_type = NVC0_QUERY_DRV_STAT(id); - info-max_value = ~0ULL; + info-max_value = 0; info-uses_byte_units = !!strstr(info-name, bytes); return 1; } else @@ -1427,15 +1427,14 @@ nvc0_screen_get_driver_query_info(struct pipe_screen *pscreen, if (screen-base.class_3d = NVE4_3D_CLASS) { info-name = nve4_pm_query_names[id - NVC0_QUERY_DRV_STAT_COUNT]; info-query_type = NVE4_PM_QUERY(id - NVC0_QUERY_DRV_STAT_COUNT); - info-max_value = (id NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) ? -~0ULL : 100; + info-max_value = (id NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) ? 0 : 100; info-uses_byte_units = FALSE; return 1; } else if (screen-compute) { info-name = nvc0_pm_query_names[id - NVC0_QUERY_DRV_STAT_COUNT]; info-query_type = NVC0_PM_QUERY(id - NVC0_QUERY_DRV_STAT_COUNT); - info-max_value = ~0ULL; + info-max_value = 0; info-uses_byte_units = FALSE; return 1; } -- 2.3.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev