[Mesa-dev] [PATCH] i965/cs: Initialize GPGPU Thread Count

[Ben Widawsky]

On Thu, Jun 11, 2015 at 09:04:45PM -0700, Jordan Justen wrote:
> This field should always be set for gen8. In the bdw PRM, Volume 2d:
> Command Reference: Structures under INTERFACE_DESCRIPTOR_DATA, DWORD
> 6, Bits 9:0, Number of Threads in GPGPU Thread Group:
> 
> "This field should not be set to 0 even if the barrier is disabled,
> since an accurate value is needed for proper pre-emption."

I am pretty skeptical that we actually need this. It's pretty clear this is a
requirement for preemption, and do we ever plan to support gpgpu preemption with
compute shaders?

Since I did some research, here's why I think it's a requirement. BDW added
supported of doing mid thread-group preemption. Doing this requires that when
the workload is restored, it has a concept of how many more threads need to
complete.

So I don't see this patch as being a requirement, but it shouldn't hurt and
probably makes looking a debug slightly easier. Also, if we do ever support
preemption, it should work. One comment inline, and then it's
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

> 
> In the HSW PRM, the it doesn't mention that it must always be set, but
> it should not hurt.
> 
> Reported-by: Kristian Høgsberg <krh at bitplanet.net>
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Kristian Høgsberg <krh at bitplanet.net>
> ---
>  src/mesa/drivers/dri/i965/brw_cs.cpp    | 19 +++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_defines.h |  5 +++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_cs.cpp b/src/mesa/drivers/dri/i965/brw_cs.cpp
> index 1f2a9d2..44c76ba 100644
> --- a/src/mesa/drivers/dri/i965/brw_cs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_cs.cpp
> @@ -284,6 +284,17 @@ brw_cs_precompile(struct gl_context *ctx,
>  }
>  
>  
> +static unsigned
> +get_cs_thread_count(const struct brw_cs_prog_data *cs_prog_data)
> +{
> +   const unsigned simd_size = cs_prog_data->simd_size;
> +   unsigned group_size = cs_prog_data->local_size[0] *
> +      cs_prog_data->local_size[1] * cs_prog_data->local_size[2];
> +
> +   return (group_size + simd_size - 1) / simd_size;
> +}
> +
> +
>  static void
>  brw_upload_cs_state(struct brw_context *brw)
>  {
> @@ -309,6 +320,8 @@ brw_upload_cs_state(struct brw_context *brw)
>                                              prog_data->binding_table.size_bytes,
>                                              32, &stage_state->bind_bo_offset);
>  
> +   unsigned threads = get_cs_thread_count(cs_prog_data);
> +
>     uint32_t dwords = brw->gen < 8 ? 8 : 9;
>     BEGIN_BATCH(dwords);
>     OUT_BATCH(MEDIA_VFE_STATE << 16 | (dwords - 2));
> @@ -358,6 +371,12 @@ brw_upload_cs_state(struct brw_context *brw)
>     desc[dw++] = 0;
>     desc[dw++] = 0;
>     desc[dw++] = stage_state->bind_bo_offset;
> +   desc[dw++] = 0;
> +   const uint32_t media_threads =
> +      brw->gen >= 8 ?
> +      SET_FIELD(threads, GEN8_MEDIA_GPGPU_THREAD_COUNT) :
> +      SET_FIELD(threads, MEDIA_GPGPU_THREAD_COUNT);
> +   desc[dw++] = media_threads;

What's the deal with, "The maximum value for global barriers is limited by the
number of threads in the system, or by 511," Can we add an assert?

>  
>     BEGIN_BATCH(4);
>     OUT_BATCH(MEDIA_INTERFACE_DESCRIPTOR_LOAD << 16 | (4 - 2));
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> index f6da305..2a8f500 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -2495,6 +2495,11 @@ enum brw_wm_barycentric_interp_mode {
>  # define MEDIA_VFE_STATE_CURBE_ALLOC_MASK       INTEL_MASK(15, 0)
>  
>  #define MEDIA_INTERFACE_DESCRIPTOR_LOAD         0x7002
> +/* GEN7 DW5, GEN8+ DW6 */
> +# define MEDIA_GPGPU_THREAD_COUNT_SHIFT         0
> +# define MEDIA_GPGPU_THREAD_COUNT_MASK          INTEL_MASK(7, 0)
> +# define GEN8_MEDIA_GPGPU_THREAD_COUNT_SHIFT    0
> +# define GEN8_MEDIA_GPGPU_THREAD_COUNT_MASK     INTEL_MASK(9, 0)
>  #define MEDIA_STATE_FLUSH                       0x7004
>  #define GPGPU_WALKER                            0x7105
>  /* GEN8+ DW2 */
> -- 
> 2.1.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev