On Fri, May 26, 2017 at 1:50 AM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On Wednesday, May 24, 2017 1:04:50 PM PDT Matt Turner wrote:
>> brw_bo_map_cpu() took a write_enable arg, but it wasn't always clear
>> whether we were also planning to read from the buffer. I kept everything
>> semantically identical by passing only MAP_READ or MAP_READ | MAP_WRITE
>> depending on the write_enable argument.
>>
>> The other flags are not used yet, but MAP_ASYNC for instance, will be
>> used in a later patch to remove the need for a separate
>> brw_bo_map_unsynchronized() function.
> [snip]
>> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h
>> b/src/mesa/drivers/dri/i965/brw_bufmgr.h
>> index 3dbde21..831da69 100644
>> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
>> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
>> @@ -173,13 +173,23 @@ void brw_bo_reference(struct brw_bo *bo);
>> */
>> void brw_bo_unreference(struct brw_bo *bo);
>>
>> +/* Must match MapBufferRange interface (for convenience) */
>> +#define MAP_READ GL_MAP_READ_BIT
>> +#define MAP_WRITE GL_MAP_WRITE_BIT
>> +#define MAP_ASYNC GL_MAP_UNSYNCHRONIZED_BIT
>> +#define MAP_PERSISTENT GL_MAP_PERSISTENT_BIT
>> +#define MAP_COHERENT GL_MAP_COHERENT_BIT
>> +/* internal */
>> +#define MAP_INTERNAL_MASK (0xff << 24)
>> +#define MAP_RAW (0x01 << 24)
>> +
>> /**
>> * Maps the buffer into userspace.
>> *
>> * This function will block waiting for any existing execution on the
>> * buffer to complete, first. The resulting mapping is returned.
>> */
>> -MUST_CHECK void *brw_bo_map_cpu(struct brw_context *brw, struct brw_bo *bo,
>> int write_enable);
>> +MUST_CHECK void *brw_bo_map_cpu(struct brw_context *brw, struct brw_bo *bo,
>> unsigned flags);
>>
>> /**
>> * Reduces the refcount on the userspace mapping of the buffer
> [snip]
>> diff --git a/src/mesa/drivers/dri/i965/intel_buffer_objects.c
>> b/src/mesa/drivers/dri/i965/intel_buffer_objects.c
>> index 090a38c..cf6382d 100644
>> --- a/src/mesa/drivers/dri/i965/intel_buffer_objects.c
>> +++ b/src/mesa/drivers/dri/i965/intel_buffer_objects.c
>> @@ -328,6 +328,13 @@ brw_map_buffer_range(struct gl_context *ctx,
>>
>> assert(intel_obj);
>>
>> + STATIC_ASSERT(GL_MAP_UNSYNCHRONIZED_BIT == MAP_ASYNC);
>> + STATIC_ASSERT(GL_MAP_WRITE_BIT == MAP_WRITE);
>> + STATIC_ASSERT(GL_MAP_READ_BIT == MAP_READ);
>> + STATIC_ASSERT(GL_MAP_PERSISTENT_BIT == MAP_PERSISTENT);
>> + STATIC_ASSERT(GL_MAP_COHERENT_BIT == MAP_COHERENT);
>> + assert((access & MAP_INTERNAL_MASK) == 0);
>> +
>> /* _mesa_MapBufferRange (GL entrypoint) sets these, but the vbo module
>> also
>> * internally uses our functions directly.
>> */
>> @@ -390,10 +397,9 @@ brw_map_buffer_range(struct gl_context *ctx,
>> alignment);
>> void *map;
>> if (brw->has_llc) {
>> - map = brw_bo_map_cpu(brw, intel_obj->range_map_bo[index],
>> - (access & GL_MAP_WRITE_BIT) != 0);
>> + map = brw_bo_map_cpu(brw, intel_obj->range_map_bo[index], access);
>> } else {
>> - map = brw_bo_map_gtt(brw, intel_obj->range_map_bo[index]);
>> + map = brw_bo_map_gtt(brw, intel_obj->range_map_bo[index], access);
>> }
>> obj->Mappings[index].Pointer = map + intel_obj->map_extra[index];
>> return obj->Mappings[index].Pointer;
>> @@ -408,10 +414,10 @@ brw_map_buffer_range(struct gl_context *ctx,
>> map = brw_bo_map_unsynchronized(brw, intel_obj->buffer);
>> } else if (!brw->has_llc && (!(access & GL_MAP_READ_BIT) ||
>> (access & GL_MAP_PERSISTENT_BIT))) {
>> - map = brw_bo_map_gtt(brw, intel_obj->buffer);
>> + map = brw_bo_map_gtt(brw, intel_obj->buffer, access);
>> mark_buffer_inactive(intel_obj);
>> } else {
>> - map = brw_bo_map_cpu(brw, intel_obj->buffer, (access &
>> GL_MAP_WRITE_BIT) != 0);
>> + map = brw_bo_map_cpu(brw, intel_obj->buffer, access);
>> mark_buffer_inactive(intel_obj);
>> }
>>
>
> One thing that's a bit odd here..."access" may contain several bits
> we don't use in brw_bo_map(), and don't #define MAP_* aliases for:
>
> GL_MAP_INVALIDATE_RANGE_BIT
> GL_MAP_INVALIDATE_BUFFER_BIT
> GL_MAP_FLUSH_EXPLICIT_BIT
>
> I guess it's harmless to pass them along...so if you think that's okay,
> I don't really have a problem with it...just thought it might be worth
> pointing out...
Yeah... seems like we can use these a lot more if and when we start
clflushing in userspace. I've put it on my TODO list.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
