On Mon, Oct 19, 2015 at 3:19 AM, Francisco Jerez <curroje...@riseup.net> wrote: > Hey Kristian, > > Kristian Høgsberg Kristensen <k...@bitplanet.net> writes: > >> We always set the mask to 0xffff, which is what it defaults to when no >> header is present. Let's drop the header instead. >> >> Signed-off-by: Kristian Høgsberg Kristensen <k...@bitplanet.net> >> --- >> src/mesa/drivers/dri/i965/brw_eu_emit.c | 3 +-- >> src/mesa/drivers/dri/i965/brw_fs.cpp | 4 ++-- >> 2 files changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c >> index bf2fee9..ebd811f 100644 >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c >> @@ -2906,11 +2906,10 @@ brw_untyped_surface_read(struct brw_codegen *p, >> const unsigned sfid = (devinfo->gen >= 8 || devinfo->is_haswell ? >> HSW_SFID_DATAPORT_DATA_CACHE_1 : >> GEN7_SFID_DATAPORT_DATA_CACHE); >> - const bool align1 = (brw_inst_access_mode(devinfo, p->current) == >> BRW_ALIGN_1); >> struct brw_inst *insn = brw_send_indirect_surface_message( >> p, sfid, dst, payload, surface, msg_length, >> brw_surface_payload_size(p, num_channels, true, true), >> - align1); >> + false); >> >> brw_set_dp_untyped_surface_read_message( >> p, insn, num_channels); >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp >> b/src/mesa/drivers/dri/i965/brw_fs.cpp >> index a2fd441..457bf59 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp >> @@ -4032,7 +4032,7 @@ fs_visitor::lower_logical_sends() >> case SHADER_OPCODE_UNTYPED_SURFACE_READ_LOGICAL: >> lower_surface_logical_send(ibld, inst, >> SHADER_OPCODE_UNTYPED_SURFACE_READ, >> - fs_reg(0xffff)); >> + fs_reg()); >> break; >> >> case SHADER_OPCODE_UNTYPED_SURFACE_WRITE_LOGICAL: >> @@ -4050,7 +4050,7 @@ fs_visitor::lower_logical_sends() >> case SHADER_OPCODE_TYPED_SURFACE_READ_LOGICAL: >> lower_surface_logical_send(ibld, inst, >> SHADER_OPCODE_TYPED_SURFACE_READ, >> - fs_reg(0xffff)); >> + fs_reg()); >> break; >> > > Whatever you do here must be in agreement with the generator and whether > it sets the header-present bit in the message descriptor or not, > otherwise you're likely to get a hang or misrendering (I guess Jenkins > would've caught this). > > However, according to my hardware docs "Typed messages (which go to the > render cache data port) must include the header.". There's no mention > of which generation that restriction applies to, but I believe it > applies to IVB/VLV *only*, which is the only generation in which typed > surface messages are handled by the render cache instead of by the data > cache -- The parenthesized comment doesn't quite make sense on newer > gens. A quick test shows no piglit regressions on HSW after removing > the header from typed surface read messages. > > I guess there are two things you could do, I'm okay with either: > > - Just drop this hunk in order to stick to the letter of the BSpec and > always pass a header together with typed surface read messages. I > have the strong suspicion though that the docs are just being > inaccurate and the header is in fact unnecessary on HSW+. No need to > resend if you decide to go down this path, if you just drop the > change for typed reads feel free to include my: > > Reviewed-by: Francisco Jerez <curroje...@riseup.net>
You're right, that should not have been there. I did get some image load/store regressions from this (on BDW as well) that I only noticed this morning. Backing out this change fixes it, thanks. Kristian > - Pass 'gen == 7 ? fs_reg(0xffff) : fs_reg()' as sample_mask argument > to lower_surface_logical_send() in the TYPED_SURFACE_READ case. For > this to work you'll also have to change the generator code to pass > the fs_inst::header_size field down to brw_typed_surface_read() so it > knows whether the header is present or not. > >> case SHADER_OPCODE_TYPED_SURFACE_WRITE_LOGICAL: >> -- >> 2.6.2 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
