[Mesa-dev] [PATCH 4/4] i965/es3.1: Implement glMemoryBarrierByRegion

[Matt Turner]

On Fri, Jul 31, 2015 at 10:25 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Fri, Jul 31, 2015 at 8:15 AM, Marta Lofstedt
> <marta.lofstedt at linux.intel.com> wrote:
>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>
>> Signed-off-by: Marta Lofstedt <marta.lofstedt at 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;
>> +   /*
>> +    * 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.
>> +   */
>> +   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)
>> +       _mesa_error(ctx, GL_INVALID_VALUE,
>> +            "glMemoryBarrierByRegion(unsupported barrier bit");
>
> It's fairly unusual to do _mesa_error() in the driver. It's done for
> some texture stuff, but in general such checking is left to the common
> implementation.
>
> Is the list of allowed bits a per-driver thing? If so, perhaps there
> should be a ctx->Const.MemoryBarriers bitfield?

I don't think so. It seems to be that exact list, according to the spec.

> And then you wouldn't even need this separate callback (at least for
> now) and just use functions->MemoryBarrier().

Oh, that's a good point.