[Mesa-dev] [PATCH 08/16] i965: Pass flags to brw_bo_map_*

[Kenneth Graunke]

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...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170526/706600e4/attachment.sig>