[Mesa-dev] [PATCH] intel: Use a URB start offset of 0 for disabled stages.

[Jordan Justen]

Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

On 2018-11-01 20:01:36, Kenneth Graunke wrote:
> There are some cases where the VS is the only stage enabled, it uses the
> entire URB, and the URB is large enough that placing later stages after
> the VS exceeds the number of bits for "URB Starting Address".
> 
> For example, on Icelake GT2, "varying-packing-simple mat2x4 array" from
> Piglit is getting a starting offset of 128 for the GS/HS/DS.  But the
> field is only large enough to hold an offset of 127.
> 
> i965 doesn't hit any genxml assertions because it's still using the old
> OUT_BATCH mechanism.  128 << GEN7_URB_STARTING_ADDRESS_SHIFT (57) == 0,
> with the extra bit falling off the end.  So we place the disabled stage
> at the beginning of the URB (overlapping with push constants).  This is
> likely okay since it's a zero size region (0 entries).
> 
> It seems like the Vulkan driver might hit this assertion, however, and
> the situation seems harmless.  To work around this, always place
> disabled stages at the start of the URB, so the last enabled stage can
> fill the remaining space without overflowing the field.
> ---
>  src/intel/common/gen_urb_config.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/intel/common/gen_urb_config.c b/src/intel/common/gen_urb_config.c
> index 0b632149cd9..1440dd713e9 100644
> --- a/src/intel/common/gen_urb_config.c
> +++ b/src/intel/common/gen_urb_config.c
> @@ -195,8 +195,14 @@ gen_get_urb_config(const struct gen_device_info *devinfo,
>     }
>  
>     /* Lay out the URB in pipeline order: push constants, VS, HS, DS, GS. */
> -   start[0] = push_constant_chunks;
> -   for (int i = MESA_SHADER_TESS_CTRL; i <= MESA_SHADER_GEOMETRY; i++) {
> -      start[i] = start[i - 1] + chunks[i - 1];
> +   int next = push_constant_chunks;
> +   for (int i = MESA_SHADER_VERTEX; i <= MESA_SHADER_GEOMETRY; i++) {
> +      if (entries[i]) {
> +         start[i] = next;
> +         next += chunks[i];
> +      } else {
> +         /* Just put disabled stages at the beginning. */
> +         start[i] = 0;
> +      }
>     }
>  }
> -- 
> 2.19.1
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev