[Mesa-dev] [PATCH 07/40] i965/blorp: Use 8k chunk size for urb allocation

[Kenneth Graunke]

On Saturday, April 16, 2016 4:42:35 PM PDT Topi Pohjolainen wrote:
> Otherwise switch from blorp to compute failes. Note that this now
> follows the normal i965 upload logic found in gen7_upload_urb().
> 
> Effectively vs_size changes from 2 to 1 and vs_start from 2 to 4.
> 
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

How about this explanation instead?

Previously, we hardcoded "VS URB Starting Address" to 2 (in 8kB chunks),
which meant VS URB data would start at an offset of 16kB.

However, on Haswell GT3 and Gen8+, we allocate the first 32kB for the
push constant region.  This means that the PS push constant and VS URB
data regions overlap, which can lead to corruption.

Cc: mesa-stable at lists.freedesktop.org

> ---
>  src/mesa/drivers/dri/i965/gen7_blorp.cpp | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp b/src/mesa/drivers/
dri/i965/gen7_blorp.cpp
> index 4debeb3..c1491ae 100644
> --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> @@ -47,8 +47,17 @@
>  static void
>  gen7_blorp_emit_urb_config(struct brw_context *brw)
>  {
> +   /* URB allocations must be done in 8k chunks. */
> +   const unsigned chunk_size_bytes = 8192;
>     const unsigned urb_size =
>        (brw->gen >= 8 || (brw->is_haswell && brw->gt == 3)) ? 32 : 16;
> +   const unsigned push_constant_bytes = 1024 * urb_size;
> +   const unsigned push_constant_chunks =
> +      push_constant_bytes / chunk_size_bytes;
> +   const unsigned vs_size = 1;

Changing vs_size from 2 to 1 seems like a separate change to me, and 
I doubt that it's necessary to fix the corruption that you're seeing.

The amount of VBO data we're uploading is 96 bytes (3 vertices * 2
attributes * 4 values * 4 bytes/float).  Each size is 512 bits = 64
bytes.  So based on the VBO data...a VS size of 2 seems right.

1 actually seems wrong, unless we don't need to account for the VUE
header (at which point it's 48 bytes, and 48 < 64 so it fits in 1.)

Does it still work if you make this "const unsigned vs_size = 2;"?

> +   const unsigned vs_start = push_constant_chunks;
> +   const unsigned vs_chunks =
> +      DIV_ROUND_UP(brw->urb.min_vs_entries * vs_size * 64, 
chunk_size_bytes);
>  
>     gen7_emit_push_constant_state(brw,
>                                   urb_size / 2 /* vs_size */,
> @@ -59,17 +68,17 @@ gen7_blorp_emit_urb_config(struct brw_context *brw)
>  
>     gen7_emit_urb_state(brw,
>                         brw->urb.min_vs_entries /* num_vs_entries */,
> -                       2 /* vs_size */,
> -                       2 /* vs_start */,
> +                       vs_size,
> +                       vs_start,
>                         0 /* num_hs_entries */,
>                         1 /* hs_size */,
> -                       2 /* hs_start */,
> +                       vs_start + vs_chunks /* hs_start */,
>                         0 /* num_ds_entries */,
>                         1 /* ds_size */,
> -                       2 /* ds_start */,
> +                       vs_start + vs_chunks /* ds_start */,
>                         0 /* num_gs_entries */,
>                         1 /* gs_size */,
> -                       2 /* gs_start */);
> +                       vs_start + vs_chunks /* gs_start */);
>  }
>  
>  
> 

-------------- 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: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160417/e9f4abfa/attachment.sig>