[Mesa-dev] [PATCH V2 21/22] i965/fs: add support for ir_txf_ms on Gen6+

[Paul Berry]

On 4 February 2013 21:48, Chris Forbes <chrisf at ijw.co.nz> wrote:

> On Gen6, lower this to `ld` with lod=0 and an extra sample_index
> parameter.
>
> On Gen7, use `ld2dms`. We don't support CMS yet for multisample
> textures, so we use MCS=0, which does the right thing for both IMS and
> UMS surfaces.
>
> Note: If we do end up emitting specialized shaders based on the MSAA
> layout, we can emit a slightly shorter message here in the UMS case.
>
> V2: Reworked completely, added support for Gen7.
>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 44
> ++++++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index d4f6fc9..a3b524c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -1061,9 +1061,17 @@ fs_visitor::emit_texture_gen5(ir_texture *ir,
> fs_reg dst, fs_reg coordinate,
>        break;
>     case ir_txf:
>        mlen = header_present + 4 * reg_width;
> +      emit(MOV(fs_reg(MRF, base_mrf + mlen - reg_width,
> BRW_REGISTER_TYPE_UD), lod));
> +      inst = emit(SHADER_OPCODE_TXF, dst);
> +      break;
> +   case ir_txf_ms:
> +      mlen = header_present + 4 * reg_width;
>
> -      emit(MOV(fs_reg(MRF, base_mrf + mlen - reg_width,
> BRW_REGISTER_TYPE_UD),
> -               lod));
> +      /* lod */
> +      emit(MOV(fs_reg(MRF, base_mrf + mlen - reg_width,
> BRW_REGISTER_TYPE_UD), fs_reg(0)));
> +      /* sample index */
> +      emit(MOV(fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_UD), lod
> /*sample*/));
>

Similar comment to the previous patch: I think this would be a lot clearer
if we went ahead and added a "sample" parameter to this function rather
than repurpose the "lod" parameter.


> +      mlen += reg_width;
>        inst = emit(SHADER_OPCODE_TXF, dst);
>        break;
>     }
> @@ -1170,16 +1178,35 @@ fs_visitor::emit_texture_gen7(ir_texture *ir,
> fs_reg dst, fs_reg coordinate,
>        mlen += reg_width;
>
>        for (int i = 1; i < ir->coordinate->type->vector_elements; i++) {
> -        emit(ADD(fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_D),
> +         emit(ADD(fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_D),
>                    coordinate, offsets[i]));
> -        coordinate.reg_offset++;
> -        mlen += reg_width;
> +         coordinate.reg_offset++;
> +         mlen += reg_width;
>

These look like unintentional diffs (perhaps due to whitespace changes).
Normally I don't get concerned about whitespace conventions, but in this
case it's making it hard to follow the patch.


> +      }
> +      break;
> +   case ir_txf_ms:
> +      emit(MOV(fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_UD), lod
> /*sample_index*/));
> +      mlen += reg_width;
> +
> +      /* constant zero MCS; we arrange to never actually have a compressed
> +       * multisample surface here for now. TODO: issue ld_mcs to get this
> first,
> +       * if we ever support texturing from compressed multisample
> surfaces */
> +      emit(MOV(fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_UD),
> fs_reg(0u)));
> +      mlen += reg_width;
> +
> +      /* there is no offsetting for this message; just copy in the integer
> +       * texture coordinates */
> +      for (int i=0; i < ir->coordinate->type->vector_elements; i++) {
> +         emit(MOV(fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_D),
> +                  coordinate));
> +         coordinate.reg_offset++;
> +         mlen += reg_width;
>        }
>        break;
>     }
>
>     /* Set up the coordinate (except for cases where it was done above) */
> -   if (ir->op != ir_txd && ir->op != ir_txs && ir->op != ir_txf) {
> +   if (ir->op != ir_txd && ir->op != ir_txs && ir->op != ir_txf && ir->op
> != ir_txf_ms) {
>        for (int i = 0; i < ir->coordinate->type->vector_elements; i++) {
>          emit(MOV(fs_reg(MRF, base_mrf + mlen), coordinate));
>          coordinate.reg_offset++;
> @@ -1195,6 +1222,7 @@ fs_visitor::emit_texture_gen7(ir_texture *ir, fs_reg
> dst, fs_reg coordinate,
>     case ir_txl: inst = emit(SHADER_OPCODE_TXL, dst); break;
>     case ir_txd: inst = emit(SHADER_OPCODE_TXD, dst); break;
>     case ir_txf: inst = emit(SHADER_OPCODE_TXF, dst); break;
> +   case ir_txf_ms: inst = emit(SHADER_OPCODE_TXF_MS, dst); break;
>     case ir_txs: inst = emit(SHADER_OPCODE_TXS, dst); break;
>     }
>     inst->base_mrf = base_mrf;
> @@ -1365,6 +1393,10 @@ fs_visitor::visit(ir_texture *ir)
>        ir->lod_info.lod->accept(this);
>        lod = this->result;
>        break;
> +   case ir_txf_ms:
> +      ir->lod_info.sample_index->accept(this);
> +      lod = this->result;
> +      break;
>     };
>
>     /* Writemasking doesn't eliminate channels on SIMD8 texture
> --
> 1.8.1.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130206/b024ab86/attachment.html>