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> - 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
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
