[Mesa-dev] [PATCH 5/5] i965: Maximum allowed size of SEND messages is 15 (4 bits)

[Kenneth Graunke]

On Friday, September 18, 2015 10:08:52 AM Iago Toral Quiroga wrote:
> Until now we only used MRFs 1..15 for regular SEND messages, so the
> message length could not possibly exceed the maximum size. Now that
> we allow to use MRF registers 1..23 in gen6, we need to be careful
> not to build messages that can go beyond the limit. That could occur,
> specifically, when building URB write messages, which we may need to
> split in chunks due to their size. Previously we would simply go and
> create a new message when we reached MRF 13 (since 13..15 were
> reserved for spilling), now we also want to check the size of the
> message explicitly.
> 
> Besides adding that condition to split URB write messages properly,
> this patch also adds asserts in the generator. Notice that
> brw_inst_set_mlen already asserts for this, but asserting in the
> generators is easy and can make debugging easier in some cases.
> ---
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 1 +
>  src/mesa/drivers/dri/i965/brw_inst.h             | 3 +++
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 1 +
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   | 5 +++--
>  4 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 6cbd42f..c65084d 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -1561,6 +1561,7 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
>        brw_set_default_exec_size(p, cvt(inst->exec_size) - 1);
>  
>        assert(inst->base_mrf + inst->mlen <= BRW_MAX_MRF(devinfo->gen));
> +      assert(inst->mlen <= BRW_MAX_MSG_LENGTH);
>  
>        switch (inst->exec_size) {
>        case 1:
> diff --git a/src/mesa/drivers/dri/i965/brw_inst.h b/src/mesa/drivers/dri/i965/brw_inst.h
> index 46eff1d..c5132ba 100644
> --- a/src/mesa/drivers/dri/i965/brw_inst.h
> +++ b/src/mesa/drivers/dri/i965/brw_inst.h
> @@ -39,6 +39,9 @@
>  extern "C" {
>  #endif
>  
> +/** Maximum SEND message length */
> +#define BRW_MAX_MSG_LENGTH 15
> +
>  /* brw_context.h has a forward declaration of brw_inst, so name the struct. */
>  typedef struct brw_inst {
>     uint64_t data[2];
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index d5943d2..05f2044 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -1136,6 +1136,7 @@ vec4_generator::generate_code(const cfg_t *cfg)
>        brw_set_default_acc_write_control(p, inst->writes_accumulator);
>  
>        assert(inst->base_mrf + inst->mlen <= BRW_MAX_MRF(devinfo->gen));
> +      assert(inst->mlen <= BRW_MAX_MSG_LENGTH);
>  
>        unsigned pre_emit_nr_insn = p->nr_insn;
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 7f06050..514ccd6 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -3320,9 +3320,10 @@ vec4_visitor::emit_vertex()
>                         prog_data->vue_map.slot_to_varying[slot]);
>  
>           /* If this was max_usable_mrf, we can't fit anything more into this
> -          * URB WRITE.
> +          * URB WRITE. Same thing if we reached the maximum length available.
>            */
> -         if (mrf > max_usable_mrf) {
> +         if (mrf > max_usable_mrf ||
> +             align_interleaved_urb_mlen(devinfo, mrf - base_mrf + 1) > BRW_MAX_MSG_LENGTH) {
>              slot++;
>              break;
>           }
> 

Doesn't this hunk need to go before patch 2?  It seems like otherwise
we'll be emitting URB write messages that are too long until patch 5.

With that fixed, the series is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Thanks for doing this, Iago!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150918/dd7887d4/attachment.sig>