On Fri, Jul 31, 2015 at 5:15 AM, Marta Lofstedt
<marta.lofst...@linux.intel.com> wrote:
> From: Marta Lofstedt <marta.lofst...@intel.com>
>
> Signed-off-by: Marta Lofstedt <marta.lofst...@intel.com>
> ---
> src/mesa/drivers/dri/i965/brw_program.c | 34
> +++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_program.c
> b/src/mesa/drivers/dri/i965/brw_program.c
> index 85e271d..332d84e 100644
> --- a/src/mesa/drivers/dri/i965/brw_program.c
> +++ b/src/mesa/drivers/dri/i965/brw_program.c
> @@ -226,6 +226,39 @@ brw_memory_barrier(struct gl_context *ctx, GLbitfield
> barriers)
> brw_emit_pipe_control_flush(brw, bits);
> }
>
> +static void
> +brw_memory_barrier_by_region(struct gl_context *ctx, GLbitfield barriers)
> +{
> + GLbitfield all_allowed_bits = GL_ATOMIC_COUNTER_BARRIER_BIT |
> + GL_FRAMEBUFFER_BARRIER_BIT |
> + GL_SHADER_IMAGE_ACCESS_BARRIER_BIT |
> + GL_SHADER_STORAGE_BARRIER_BIT |
> + GL_TEXTURE_FETCH_BARRIER_BIT |
> + GL_UNIFORM_BARRIER_BIT;
Indent these to match GL_ATOMIC_COUNTER_BARRIER_BIT.
Also, GL_ATOMIC_COUNTER_BARRIER_BIT and GL_SHADER_STORAGE_BARRIER_BIT
are not handled by brw_memory_barrier(). I know the latter is part of
an in-progress feature, but are we missing something for atomic
counters?
> + /*
> + * According to OpenGL ES 3.1 spec. April 29, 2015, 7.11.2:
> + * "When barriers are ALL_BARRIERS_BIT, shader memory access
> + * will be synchronized realtive to all theese barrier bits,
> + * but not to other barrier bits specific to MemoryBarrier."
Just copy and paste from the spec to avoid s/is/are/ and typos.
> + * I.e if bariiers is the special value GL_ALL_BARRIER_BITS,
> + * then all barriers allowed by glMemoryBarrierByRegion
> + * should be activated.
> + */
> + if (barriers == GL_ALL_BARRIER_BITS)
> + return brw_memory_barrier(ctx, all_allowed_bits);
> +
> + /*
> + * If barriers contain a value that is not allowed
> + * for glMemoryBarrierByRegion an GL_INVALID_VALUE
> + * should be generated.
> + */
> + if ((all_allowed_bits | barriers) ^ all_allowed_bits)
This might work but I have a hard time thinking about it. The usual pattern is
(barriers & ~all_allowed_bits) != 0
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "glMemoryBarrierByRegion(unsupported barrier bit");
Indent.
I've attached a patch that cleans up the things I mentioned. Please
squash it in. With that, the patch looks good. I suppose the only
pending question is about the unhandled GL_ATOMIC_COUNTER_BARRIER_BIT
bit.
diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
index 332d84e..eb8ae3f 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -230,31 +230,34 @@ static void
brw_memory_barrier_by_region(struct gl_context *ctx, GLbitfield barriers)
{
GLbitfield all_allowed_bits = GL_ATOMIC_COUNTER_BARRIER_BIT |
- GL_FRAMEBUFFER_BARRIER_BIT |
- GL_SHADER_IMAGE_ACCESS_BARRIER_BIT |
- GL_SHADER_STORAGE_BARRIER_BIT |
- GL_TEXTURE_FETCH_BARRIER_BIT |
- GL_UNIFORM_BARRIER_BIT;
- /*
- * According to OpenGL ES 3.1 spec. April 29, 2015, 7.11.2:
- * "When barriers are ALL_BARRIERS_BIT, shader memory access
- * will be synchronized realtive to all theese barrier bits,
- * but not to other barrier bits specific to MemoryBarrier."
- * I.e if bariiers is the special value GL_ALL_BARRIER_BITS,
- * then all barriers allowed by glMemoryBarrierByRegion
- * should be activated.
- */
+ GL_FRAMEBUFFER_BARRIER_BIT |
+ GL_SHADER_IMAGE_ACCESS_BARRIER_BIT |
+ GL_SHADER_STORAGE_BARRIER_BIT |
+ GL_TEXTURE_FETCH_BARRIER_BIT |
+ GL_UNIFORM_BARRIER_BIT;
+
+ /* From section 7.11.2 of the OpenGL ES 3.1 specification:
+ *
+ * "When barriers is ALL_BARRIER_BITS, shader memory accesses will be
+ * synchronized relative to all these barrier bits, but not to other
+ * barrier bits specific to MemoryBarrier."
+ *
+ * That is, if barriers is the special value GL_ALL_BARRIER_BITS, then all
+ * barriers allowed by glMemoryBarrierByRegion should be activated."
+ */
if (barriers == GL_ALL_BARRIER_BITS)
return brw_memory_barrier(ctx, all_allowed_bits);
- /*
- * If barriers contain a value that is not allowed
- * for glMemoryBarrierByRegion an GL_INVALID_VALUE
- * should be generated.
- */
- if ((all_allowed_bits | barriers) ^ all_allowed_bits)
+ /* From section 7.11.2 of the OpenGL ES 3.1 specification:
+ *
+ * "An INVALID_VALUE error is generated if barriers is not the special
+ * value ALL_BARRIER_BITS, and has any bits set other than those
+ * described above."
+ */
+ if ((barriers & ~all_allowed_bits) != 0) {
_mesa_error(ctx, GL_INVALID_VALUE,
- "glMemoryBarrierByRegion(unsupported barrier bit");
+ "glMemoryBarrierByRegion(unsupported barrier bit");
+ }
return brw_memory_barrier(ctx, barriers);
}
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
