On Tue, Oct 20, 2015 at 3:25 PM, Ben Widawsky <b...@bwidawsk.net> wrote:
> On Tue, Oct 20, 2015 at 03:17:38PM -0700, Matt Turner wrote:
>> On Tue, Oct 20, 2015 at 2:29 PM, Ben Widawsky
>> <benjamin.widaw...@intel.com> wrote:
>> > This patch is split out for review. It will be squashed before pushing.
>> > ---
>> > src/mesa/drivers/dri/i965/brw_defines.h | 1 +
>> > src/mesa/drivers/dri/i965/brw_fs.cpp | 6 +++-
>> > src/mesa/drivers/dri/i965/brw_fs.h | 2 ++
>> > src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 44
>> > ++++++++++++++++++++++++++
>> > src/mesa/drivers/dri/i965/brw_shader.cpp | 2 ++
>> > 5 files changed, 54 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
>> > b/src/mesa/drivers/dri/i965/brw_defines.h
>> > index c67728b..2c5cd7a 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_defines.h
>> > +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>> > @@ -915,6 +915,7 @@ enum opcode {
>> > */
>> > FS_OPCODE_FB_WRITE_LOGICAL,
>> >
>> > + FS_OPCODE_PACK_STENCIL_REF,
>>
>> Listing this in the middle of four fbwrite opcodes seems wrong.
>>
>
> Can you please find me a better place? I searched all over and really wasn't
> sure where it belongs. It is FB write related (unless there is a more generic
> use of such packing), but it is indeed not an FB write.
Immediately after the fbwrites would be better. Or how about above
VEC4_OPCODE_MOV_BYTES? The block started by VEC4_OPCODE_MOV_BYTES all
do regioned MOVs similar to this opcode?
My only concern was putting it in the middle of a group of opcodes it
doesn't belong to.
>> > FS_OPCODE_BLORP_FB_WRITE,
>> > FS_OPCODE_REP_FB_WRITE,
>> > SHADER_OPCODE_RCP,
>> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> > b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> > index 560eb91..c962043 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> > @@ -3440,7 +3440,11 @@ lower_fb_write_logical_send(const fs_builder &bld,
>> > fs_inst *inst,
>> > if (src_stencil.file != BAD_FILE) {
>> > assert(devinfo->gen >= 9);
>> > assert(bld.dispatch_width() != 16);
>> > - sources[length] = src_stencil;
>> > +
>> > + sources[length] = bld.vgrf(BRW_REGISTER_TYPE_UD);
>> > + bld.exec_all().annotate("FB write OS")
>>
>> OS?
>>
>
> "Output Stencil" It's the common form in all the payload definitions in the
> docs. I kind of liked keeping the doc term so it's searchable, even though we
> hadn't done that for other things like oMask (should be OM). I can rename it
> to
> whatever you like instead - just tell me what you want.
I don't have a preference. I just wouldn't have any idea what the
annotation meant if I saw it in INTEL_DEBUG=fs output, which seems
like it reduces the usefulness.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
