[Mesa-dev] [PATCH 07/13] gallium: add interface for persistent and coherent buffer mappings

Marek Olšák maraeo at gmail.com
Thu Jan 30 09:42:01 PST 2014


On Thu, Jan 30, 2014 at 6:34 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 30.01.2014 17:50, schrieb Marek Olšák:
>> On Thu, Jan 30, 2014 at 5:31 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>>> Am 30.01.2014 02:20, schrieb Marek Olšák:
>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>
>>>> Required for ARB_buffer_storage.
>>>> ---
>>>>  src/gallium/docs/source/context.rst    | 22 ++++++++++++++++++++++
>>>>  src/gallium/docs/source/screen.rst     |  3 +++
>>>>  src/gallium/drivers/trace/tr_context.c | 16 ++++++++++++++++
>>>>  src/gallium/include/pipe/p_context.h   |  7 ++++++-
>>>>  src/gallium/include/pipe/p_defines.h   | 31 +++++++++++++++++++++++++++++--
>>>>  5 files changed, 76 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/gallium/docs/source/context.rst b/src/gallium/docs/source/context.rst
>>>> index 1037162..4258e43 100644
>>>> --- a/src/gallium/docs/source/context.rst
>>>> +++ b/src/gallium/docs/source/context.rst
>>>> @@ -520,6 +520,16 @@ invalidates all read caches of the currently-set samplers.
>>>>
>>>>
>>>>
>>>> +.. _memory_barrier:
>>>> +
>>>> +memory_barrier
>>>> +%%%%%%%%%%%%%%%
>>>> +
>>>> +This function flushes caches according to which of the PIPE_BARRIER_* flags
>>>> +are set.
>>>> +
>>>> +
>>>> +
>>>>  .. _pipe_transfer:
>>>>
>>>>  PIPE_TRANSFER
>>>> @@ -557,6 +567,18 @@ These flags control the behavior of a transfer object.
>>>>    Written ranges will be notified later with :ref:`transfer_flush_region`.
>>>>    Cannot be used with ``PIPE_TRANSFER_READ``.
>>>>
>>>> +``PIPE_TRANSFER_PERSISTENT``
>>>> +  Allows the resource to be used for rendering while mapped.
>>>> +  PIPE_RESOURCE_FLAG_TRANSFER_PERSISTENT must be set when creating
>>>> +  the resource.
>>>> +  If COHERENT is not set, memory_barrier(PIPE_BARRIER_MAPPED_BUFFER)
>>>> +  must be called to ensure the device can see what the CPU has written.
>>>> +
>>>> +``PIPE_TRANSFER_COHERENT``
>>>> +  If PERSISTENT is set, this ensures any writes done by the device are
>>>> +  immediately visible to the CPU and vice versa.
>>>> +  PIPE_RESOURCE_FLAG_TRANSFER_COHERENT must be set when creating
>>>> +  the resource.
>>>>
>>>>  Compute kernel execution
>>>>  ^^^^^^^^^^^^^^^^^^^^^^^^
>>>> diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
>>>> index ed8e832..cbb07d6 100644
>>>> --- a/src/gallium/docs/source/screen.rst
>>>> +++ b/src/gallium/docs/source/screen.rst
>>>> @@ -176,6 +176,9 @@ The integer capabilities:
>>>>    ARB_framebuffer_object is provided.
>>>>  * ``PIPE_CAP_TGSI_VS_LAYER``: Whether TGSI_SEMANTIC_LAYER is supported
>>>>    as a vertex shader output.
>>>> +* ``PIPE_CAP_BUFFER_TRANSFER_PERSISTENT_COHERENT``: Whether
>>>> +  PIPE_TRANSFER_PERSISTENT and PIPE_TRANSFER_COHERENT are supported
>>>> +  for buffers.
>>>>
>>>>
>>>>  .. _pipe_capf:
>>>> diff --git a/src/gallium/drivers/trace/tr_context.c b/src/gallium/drivers/trace/tr_context.c
>>>> index 60ebea4..c10e010 100644
>>>> --- a/src/gallium/drivers/trace/tr_context.c
>>>> +++ b/src/gallium/drivers/trace/tr_context.c
>>>> @@ -1489,6 +1489,21 @@ static void trace_context_texture_barrier(struct pipe_context *_context)
>>>>  }
>>>>
>>>>
>>>> +static void trace_context_memory_barrier(struct pipe_context *_context,
>>>> +                                         unsigned flags)
>>>> +{
>>>> +   struct trace_context *tr_context = trace_context(_context);
>>>> +   struct pipe_context *context = tr_context->pipe;
>>>> +
>>>> +   trace_dump_call_begin("pipe_context", "memory_barrier");
>>>> +   trace_dump_arg(ptr, context);
>>>> +   trace_dump_arg(uint, flags);
>>>> +   trace_dump_call_end();
>>>> +
>>>> +   context->memory_barrier(context, flags);
>>>> +}
>>>> +
>>>> +
>>>>  static const struct debug_named_value rbug_blocker_flags[] = {
>>>>     {"before", 1, NULL},
>>>>     {"after", 2, NULL},
>>>> @@ -1577,6 +1592,7 @@ trace_context_create(struct trace_screen *tr_scr,
>>>>     TR_CTX_INIT(clear_depth_stencil);
>>>>     TR_CTX_INIT(flush);
>>>>     TR_CTX_INIT(texture_barrier);
>>>> +   TR_CTX_INIT(memory_barrier);
>>>>
>>>>     TR_CTX_INIT(transfer_map);
>>>>     TR_CTX_INIT(transfer_unmap);
>>>> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
>>>> index 8ef6e27..93f4c57 100644
>>>> --- a/src/gallium/include/pipe/p_context.h
>>>> +++ b/src/gallium/include/pipe/p_context.h
>>>> @@ -406,7 +406,12 @@ struct pipe_context {
>>>>      * Flush any pending framebuffer writes and invalidate texture caches.
>>>>      */
>>>>     void (*texture_barrier)(struct pipe_context *);
>>>> -
>>>> +
>>>> +   /**
>>>> +    * Flush caches according to flags.
>>>> +    */
>>>> +   void (*memory_barrier)(struct pipe_context *, unsigned flags);
>>>> +
>>>>     /**
>>>>      * Creates a video codec for a specific video format/profile
>>>>      */
>>>> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
>>>> index 8d08a90..31a9c72 100644
>>>> --- a/src/gallium/include/pipe/p_defines.h
>>>> +++ b/src/gallium/include/pipe/p_defines.h
>>>> @@ -295,8 +295,27 @@ enum pipe_transfer_usage {
>>>>      * - D3D10 DDI's D3D10_DDI_MAP_WRITE_DISCARD flag
>>>>      * - D3D10's D3D10_MAP_WRITE_DISCARD flag.
>>>>      */
>>>> -   PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE = (1 << 12)
>>>> +   PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE = (1 << 12),
>>>>
>>>> +   /**
>>>> +    * Allows the resource to be used for rendering while mapped.
>>>> +    *
>>>> +    * PIPE_RESOURCE_FLAG_TRANSFER_PERSISTENT must be set when creating
>>>> +    * the resource.
>>>> +    *
>>>> +    * If COHERENT is not set, memory_barrier(PIPE_BARRIER_MAPPED_BUFFER)
>>>> +    * must be called to ensure the device can see what the CPU has written.
>>>> +    */
>>>> +   PIPE_TRANSFER_PERSISTENT = (1 << 13),
>>>> +
>>>> +   /**
>>>> +    * If PERSISTENT is set, this ensures any writes done by the device are
>>>> +    * immediately visible to the CPU and vice versa.
>>>> +    *
>>>> +    * PIPE_RESOURCE_FLAG_TRANSFER_COHERENT must be set when creating
>>>> +    * the resource.
>>>> +    */
>>>> +   PIPE_TRANSFER_COHERENT = (1 << 14)
>>> Here and in docs, what's the effect if persistent isn't set? Does that
>>> just have no effect?
>>
>> Yes, it has no effect.
>>
>>>
>>>
>>>>  };
>>>>
>>>>  /**
>>>> @@ -306,6 +325,11 @@ enum pipe_flush_flags {
>>>>     PIPE_FLUSH_END_OF_FRAME = (1 << 0)
>>>>  };
>>>>
>>>> +/**
>>>> + * Flags for pipe_context::memory_barrier.
>>>> + */
>>>> +#define PIPE_BARRIER_MAPPED_BUFFER     (1 << 0)
>>>> +
>>>>  /*
>>>>   * Resource binding flags -- state tracker must specify in advance all
>>>>   * the ways a resource might be used.
>>>> @@ -353,6 +377,8 @@ enum pipe_flush_flags {
>>>>  /* Flags for the driver about resource behaviour:
>>>>   */
>>>>  #define PIPE_RESOURCE_FLAG_GEN_MIPS    (1 << 0)  /* Driver performs autogen mips */
>>>> +#define PIPE_RESOURCE_FLAG_TRANSFER_PERSISTENT (1 << 1)
>>>> +#define PIPE_RESOURCE_FLAG_TRANSFER_COHERENT   (1 << 2)
>>>>  #define PIPE_RESOURCE_FLAG_DRV_PRIV    (1 << 16) /* driver/winsys private */
>>>>  #define PIPE_RESOURCE_FLAG_ST_PRIV     (1 << 24) /* state-tracker/winsys private */
>>>>
>>>> @@ -521,7 +547,8 @@ enum pipe_cap {
>>>>     PIPE_CAP_MAX_VIEWPORTS = 84,
>>>>     PIPE_CAP_ENDIANNESS = 85,
>>>>     PIPE_CAP_MIXED_FRAMEBUFFER_SIZES = 86,
>>>> -   PIPE_CAP_TGSI_VS_LAYER = 87
>>>> +   PIPE_CAP_TGSI_VS_LAYER = 87,
>>>> +   PIPE_CAP_BUFFER_TRANSFER_PERSISTENT_COHERENT = 88
>>>>  };
>>>>
>>>>  #define PIPE_QUIRK_TEXTURE_BORDER_COLOR_SWIZZLE_NV50 (1 << 0)
>>>>
>>>
>>> Other than that, this interface change looks alright to me. I guess the
>>> "transfer" name is a bit of a misnomer since transfer objects could be
>>> copies, which doesn't make much sense with persistent and coherent
>>> objects, but I can't see how you could change that.
>>
>> I guess I could change the names to:
>> PIPE_RESOURCE_FLAG_MAP_PERSISTENT
>> PIPE_RESOURCE_FLAG_MAP_COHERENT
>> PIPE_CAP_MAP_BUFFER_PERSISTENT_COHERENT
>>
>> Marek
>>
>
> Maybe it would be better. You still have to get a "transfer" object
> first though, but you can't change that. I dunno either way is fine by me.

No, we get a transfer object when mapping a buffer. There is no
separate "create" function anymore.

Marek


More information about the mesa-dev mailing list