[Mesa-dev] [PATCH 2/3] i965/fs: Generate LOD sampler message from ir_lod.

Kenneth Graunke kenneth at whitecape.org
Wed Mar 27 15:31:14 PDT 2013


On 03/19/2013 11:51 AM, Matt Turner wrote:
> ---
>   src/mesa/drivers/dri/i965/brw_defines.h        |    2 ++
>   src/mesa/drivers/dri/i965/brw_fs.cpp           |    4 +++-
>   src/mesa/drivers/dri/i965/brw_fs_emit.cpp      |    6 ++++++
>   src/mesa/drivers/dri/i965/brw_fs_visitor.cpp   |    9 +++++++++
>   src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |    3 +++
>   5 files changed, 23 insertions(+), 1 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> index d9b7f9a..24a1f00 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -710,6 +710,7 @@ enum opcode {
>      SHADER_OPCODE_TXS,
>      FS_OPCODE_TXB,
>      SHADER_OPCODE_TXF_MS,
> +   SHADER_OPCODE_LOD,
>
>      SHADER_OPCODE_SHADER_TIME_ADD,
>
> @@ -894,6 +895,7 @@ enum brw_message_target {
>   #define GEN5_SAMPLER_MESSAGE_SAMPLE_BIAS_COMPARE 5
>   #define GEN5_SAMPLER_MESSAGE_SAMPLE_LOD_COMPARE  6
>   #define GEN5_SAMPLER_MESSAGE_SAMPLE_LD           7
> +#define GEN6_SAMPLER_MESSAGE_LOD                 9

This message is actually available on Ironlake too, AFAICT, so why not 
GEN5_SAMPLER_MESSAGE_LOD?

>   #define GEN5_SAMPLER_MESSAGE_SAMPLE_RESINFO      10
>   #define HSW_SAMPLER_MESSAGE_SAMPLE_DERIV_COMPARE 20
>   #define GEN7_SAMPLER_MESSAGE_SAMPLE_LD_MCS       29
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 927cf13..4d2e17c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -338,7 +338,8 @@ fs_inst::is_tex()
>              opcode == SHADER_OPCODE_TXF ||
>              opcode == SHADER_OPCODE_TXF_MS ||
>              opcode == SHADER_OPCODE_TXL ||
> -           opcode == SHADER_OPCODE_TXS);
> +           opcode == SHADER_OPCODE_TXS ||
> +           opcode == SHADER_OPCODE_LOD);
>   }
>
>   bool
> @@ -744,6 +745,7 @@ fs_visitor::implied_mrf_writes(fs_inst *inst)
>      case SHADER_OPCODE_TXF_MS:
>      case SHADER_OPCODE_TXL:
>      case SHADER_OPCODE_TXS:
> +   case SHADER_OPCODE_LOD:
>         return 1;
>      case SHADER_OPCODE_SHADER_TIME_ADD:
>         return 0;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
> index 2391ad1..039589c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
> @@ -404,6 +404,11 @@ fs_generator::generate_tex(fs_inst *inst, struct brw_reg dst, struct brw_reg src
>            else
>               msg_type = GEN5_SAMPLER_MESSAGE_SAMPLE_LD;
>            break;
> +      case SHADER_OPCODE_LOD:
> +         /* Gen6+. Otherwise ARB_texture_query_lod not exposed. */
> +         assert(intel->gen >= 6);
> +         msg_type = GEN6_SAMPLER_MESSAGE_LOD;

It seems trivial to support on Ironlake (literally change your assert), 
so why not?

> +         break;
>         default:
>   	 assert(!"not reached");
>   	 break;
> @@ -1245,6 +1250,7 @@ fs_generator::generate_code(exec_list *instructions)
>         case SHADER_OPCODE_TXF_MS:
>         case SHADER_OPCODE_TXL:
>         case SHADER_OPCODE_TXS:
> +      case SHADER_OPCODE_LOD:
>   	 generate_tex(inst, dst, src[0]);
>   	 break;
>         case FS_OPCODE_DDX:
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 92bc621..1d744d1 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -944,6 +944,9 @@ fs_visitor::emit_texture_gen4(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>      case ir_txf:
>         inst = emit(SHADER_OPCODE_TXF, dst);
>         break;
> +   case ir_lod:
> +      inst = emit(SHADER_OPCODE_LOD, dst);
> +      break;

If you aren't supporting the extension on Gen4, why do you have code for 
it here?

>      default:
>         fail("unrecognized texture opcode");
>      }
> @@ -1084,6 +1087,9 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>         mlen += reg_width;
>         inst = emit(SHADER_OPCODE_TXF_MS, dst);
>         break;
> +   case ir_lod:
> +      inst = emit(SHADER_OPCODE_LOD, dst);
> +      break;
>      }
>      inst->base_mrf = base_mrf;
>      inst->mlen = mlen;
> @@ -1124,6 +1130,7 @@ fs_visitor::emit_texture_gen7(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>      /* Set up the LOD info */
>      switch (ir->op) {
>      case ir_tex:
> +   case ir_lod:
>         break;
>      case ir_txb:
>         emit(MOV(fs_reg(MRF, base_mrf + mlen), lod));
> @@ -1237,6 +1244,7 @@ fs_visitor::emit_texture_gen7(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>      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;
> +   case ir_lod: inst = emit(SHADER_OPCODE_LOD, dst); break;
>      }
>      inst->base_mrf = base_mrf;
>      inst->mlen = mlen;
> @@ -1388,6 +1396,7 @@ fs_visitor::visit(ir_texture *ir)
>      fs_reg lod, lod2, sample_index;
>      switch (ir->op) {
>      case ir_tex:
> +   case ir_lod:
>         break;
>      case ir_txb:
>         ir->lod_info.bias->accept(this);
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 32de82b..3318d6f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -2127,6 +2127,7 @@ vec4_visitor::visit(ir_texture *ir)
>         lod_type = ir->lod_info.grad.dPdx->type;
>         break;
>      case ir_txb:
> +   case ir_lod:
>         break;
>      }
>
> @@ -2150,6 +2151,8 @@ vec4_visitor::visit(ir_texture *ir)
>         break;
>      case ir_txb:
>         assert(!"TXB is not valid for vertex shaders.");

I'd like to see a "break;" here.

> +   case ir_lod:
> +      assert(!"LOD is not valid for vertex shaders.");
>      }
>
>      bool use_texture_offset = ir->offset != NULL && ir->op != ir_txf;

Assuming you add support for Ironlake here and in patch 3, this series is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>


More information about the mesa-dev mailing list