[Mesa-dev] [PATCH v2 05/25] gallium: add sparse buffer interface and capability

[Marek Olšák]

On Wed, Mar 29, 2017 at 12:26 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 28.03.2017 21:46, Marek Olšák wrote:
>>
>> On Tue, Mar 28, 2017 at 11:11 AM, Nicolai Hähnle <nhaehnle at gmail.com>
>> wrote:
>>>
>>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>>
>>> TODO fill out caps in all drivers
>>>
>>> v2:
>>> - explain the resource_commit interface in more detail
>>> ---
>>>  src/gallium/docs/source/context.rst  | 25 +++++++++++++++++++++++++
>>>  src/gallium/docs/source/screen.rst   |  3 +++
>>>  src/gallium/include/pipe/p_context.h | 13 +++++++++++++
>>>  src/gallium/include/pipe/p_defines.h |  2 ++
>>>  4 files changed, 43 insertions(+)
>>>
>>> diff --git a/src/gallium/docs/source/context.rst
>>> b/src/gallium/docs/source/context.rst
>>> index a053193..5949ff2 100644
>>> --- a/src/gallium/docs/source/context.rst
>>> +++ b/src/gallium/docs/source/context.rst
>>> @@ -611,20 +611,45 @@ for both regular textures as well as for
>>> framebuffers read via FBFETCH.
>>>  .. _memory_barrier:
>>>
>>>  memory_barrier
>>>  %%%%%%%%%%%%%%%
>>>
>>>  This function flushes caches according to which of the PIPE_BARRIER_*
>>> flags
>>>  are set.
>>>
>>>
>>>
>>> +.. _resource_commit:
>>> +
>>> +resource_commit
>>> +%%%%%%%%%%%%%%%
>>> +
>>> +This function changes the commit state of a part of a sparse resource.
>>> Sparse
>>> +resources are created by setting the ``PIPE_RESOURCE_FLAG_SPARSE`` flag
>>> when
>>> +calling ``resource_create``. Initially, sparse resources only reserve a
>>> virtual
>>> +memory region that is not backed by memory (i.e., it is uncommitted).
>>> The
>>> +``resource_commit`` function can be called to commit or uncommit parts
>>> (or all)
>>> +of a resource. The driver manages the underlying backing memory.
>>> +
>>> +The contents of newly committed memory regions are undefined. Calling
>>> this
>>> +function to commit an already committed memory region is allowed and
>>> leaves its
>>> +content unchanged. Similarly, calling this function to uncommit an
>>> already
>>> +uncommitted memory region is allowed.
>>> +
>>> +For buffers, the given box must be aligned to multiples of
>>> +``PIPE_CAP_SPARSE_BUFFER_PAGE_SIZE``. As an exception to this rule, if
>>> the size
>>> +of the buffer is not a multiple of the page size, changing the commit
>>> state of
>>> +the last (partial) page requires a box that ends at the end of the
>>> buffer
>>> +(i.e., box->x + box->width == buffer->width0).
>>> +
>>> +
>>> +
>>>  .. _pipe_transfer:
>>>
>>>  PIPE_TRANSFER
>>>  ^^^^^^^^^^^^^
>>>
>>>  These flags control the behavior of a transfer object.
>>>
>>>  ``PIPE_TRANSFER_READ``
>>>    Resource contents read back (or accessed directly) at transfer create
>>> time.
>>>
>>> diff --git a/src/gallium/docs/source/screen.rst
>>> b/src/gallium/docs/source/screen.rst
>>> index 00c9503..8759639 100644
>>> --- a/src/gallium/docs/source/screen.rst
>>> +++ b/src/gallium/docs/source/screen.rst
>>> @@ -369,20 +369,23 @@ The integer capabilities:
>>>    opcode to retrieve the current value in the framebuffer.
>>>  * ``PIPE_CAP_TGSI_MUL_ZERO_WINS``: Whether TGSI shaders support the
>>>    ``TGSI_PROPERTY_MUL_ZERO_WINS`` shader property.
>>>  * ``PIPE_CAP_DOUBLES``: Whether double precision floating-point
>>> operations
>>>    are supported.
>>>  * ``PIPE_CAP_INT64``: Whether 64-bit integer operations are supported.
>>>  * ``PIPE_CAP_INT64_DIVMOD``: Whether 64-bit integer division/modulo
>>>    operations are supported.
>>>  * ``PIPE_CAP_TGSI_TEX_TXF_LZ``: Whether TEX_LZ and TXF_LZ opcodes are
>>>    supported.
>>> +* ``PIPE_CAP_SPARSE_BUFFER_PAGE_SIZE``: The page size of sparse buffers
>>> in
>>> +  bytes, or 0 if sparse buffers are not supported. The page size must be
>>> at
>>> +  most 64KB.
>>>
>>>
>>>  .. _pipe_capf:
>>>
>>>  PIPE_CAPF_*
>>>  ^^^^^^^^^^^^^^^^
>>>
>>>  The floating-point capabilities are:
>>>
>>>  * ``PIPE_CAPF_MAX_LINE_WIDTH``: The maximum width of a regular line.
>>> diff --git a/src/gallium/include/pipe/p_context.h
>>> b/src/gallium/include/pipe/p_context.h
>>> index a29fff5..4d5535b 100644
>>> --- a/src/gallium/include/pipe/p_context.h
>>> +++ b/src/gallium/include/pipe/p_context.h
>>> @@ -578,20 +578,33 @@ struct pipe_context {
>>>      * Flush any pending framebuffer writes and invalidate texture
>>> caches.
>>>      */
>>>     void (*texture_barrier)(struct pipe_context *, unsigned flags);
>>>
>>>     /**
>>>      * Flush caches according to flags.
>>>      */
>>>     void (*memory_barrier)(struct pipe_context *, unsigned flags);
>>>
>>>     /**
>>> +    * Change the commitment status of a part of the given resource,
>>> which must
>>> +    * have been created with the PIPE_RESOURCE_FLAG_SPARSE bit.
>>> +    *
>>> +    * \param level The texture level whose commitment should be changed.
>>> +    * \param box The region of the resource whose commitment should be
>>> changed.
>>> +    * \param commit Whether memory should be committed or un-committed.
>>> +    *
>>> +    * \return false if out of memory, true on success.
>>> +    */
>>> +   bool (*resource_commit)(struct pipe_context *, struct pipe_resource
>>> *,
>>> +                           unsigned level, struct pipe_box *box, bool
>>> commit);
>>
>>
>> I wonder what the behavior for threaded gallium should be. Possibilities:
>> 1) Sync the context thread and execute directly.
>> 2) Ignore the return value, always return true, and execute it
>> asynchronously.
>>
>> If the "false" return value is very unlikely, I may use the second
>> approach.
>
>
> "false" here means out-of-memory, or some error returned by the kernel on
> the map operations (which in practice also should always be
> out-of-memory...). What's the story here for threaded gallium? We're not
> exactly consistent in reporting out-of-memory errors anyway (on draw calls
> especially), but maybe that's something to improve...

Well, for example, threaded gallium ignores the return values of
begin_query and end_query. Those are pretty unlikely to fail. I wonder
if this is similar. As long as we can't get a failure due to invalid
parameters, it should be OK to ignore the out-of-memory error. The
idea is that games usually shouldn't get out-of-memory errors and if
they do, they can't deal with them.

Marek