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

Wed Feb 8 19:15:17 UTC 2017

On 08.02.2017 18:16, Roland Scheidegger wrote:
> Am 08.02.2017 um 13:42 schrieb Nicolai Hähnle:
>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>
>> TODO fill out caps in all drivers
>> ---
>>  src/gallium/docs/source/context.rst  |  9 +++++++++
>>  src/gallium/docs/source/screen.rst   |  3 +++
>>  src/gallium/include/pipe/p_context.h | 13 +++++++++++++
>>  src/gallium/include/pipe/p_defines.h |  2 ++
>>  src/gallium/include/pipe/p_screen.h  |  1 -
>>  5 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/docs/source/context.rst b/src/gallium/docs/source/context.rst
>> index d8b2560..b47b75b 100644
>> --- a/src/gallium/docs/source/context.rst
>> +++ b/src/gallium/docs/source/context.rst
>> @@ -593,6 +593,15 @@ are set.
>>
>>
>>
>> +.. _resource_commit:
>> +
>> +resource_commit
>> +%%%%%%%%%%%%%%%
>> +
>> +This function changes the commitment of a part of a sparse resource.
> This is quite brief...

It is :)

I can add some of this discussion in a v2.

>
>> +
>> +
>> +
>>  .. _pipe_transfer:
>>
>>  PIPE_TRANSFER
>> diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
>> index 4f5b4bb..e8aeb2e 100644
>> --- a/src/gallium/docs/source/screen.rst
>> +++ b/src/gallium/docs/source/screen.rst
>> @@ -376,6 +376,9 @@ The integer capabilities:
>>  * ``PIPE_CAP_DOUBLES``: Whether double precision floating-point operations
>>    are supported.
>>  * ``PIPE_CAP_INT64``: Whether 64-bit integer operations 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:
>> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
>> index 45098c9..85a80d7 100644
>> --- a/src/gallium/include/pipe/p_context.h
>> +++ b/src/gallium/include/pipe/p_context.h
>> @@ -574,6 +574,19 @@ struct pipe_context {
>>     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 suppose it makes sense that this operates on the context level?

Yes.

ARB_sparse_{texture,buffer} are actually not clear on how multiple 
contexts should interact (I think there are plans to clarify the language).

What is clear is that BufferPageCommitment calls are tied to a context, 
and should be properly sequenced against other calls in the same context.

Therefore, this doesn't work as a pipe_screen function: e.g. for 
radeonsi, we need to flush the command buffer when this function is 
called (virtual memory mapping calls cannot be added to a command buffer).

> Should something be said about alignment of the box? I'd guess as long
> as this is only for buffers, x/width need to be aligned to page
> boundaries (taking the format into consideration, but I suppose not a
> problem for buffers), but I don't really see how that would work with
> textures (small mips etc.).

For buffers, this should indeed be page aligned (where the page size 
comes from the PIPE_CAP). A special case is that x + width == res->width 
is allowed, i.e. res->width need not be a multiple of the page size.

For textures, the story is much more complicated, but I think the 
interface in this patch is forward compatible.

ARB_sparse_texture extends the GL internalformat query functions to 
allow querying for different possible texture "page" layouts (which can 
be different depending on the internalformat; each internalformat can 
have multiple layouts, e.g. 256x128x1 and 128x256x1). This will need a 
new pipe_screen interface, I believe.

The app chooses the sparse layout and can then selectively commit boxes 
per level that are aligned to the page size of the chosen layout, for a 
prefix of the mip tree. Beyond that prefix lies the mip-tail, which is 
committed all-or-nothing. IIRC the driver gets to choose at texture 
creation time how many levels are sparse and how many are covered by the 
mip-tail.

So we'd need to add fields for sparse layout index and number of sparse 
levels to pipe_resource. resource_commit with level set to >= number of 
sparse level would affect the mip-tail. This matches the GL behavior IIRC.

The all-or-nothing for the mip-tail means that for texture views that 
cover part of the mip-tail, calling the commit functions actually 
affects the whole mip-tail of the underlying texture.

This series isn't about textures at all, but I guess it's good to know 
for context :)

Cheers,
Nicolai

>
> Roland
>
>
>
>> +
>> +   /**
>>      * Creates a video codec for a specific video format/profile
>>      */
>>     struct pipe_video_codec *(*create_video_codec)( struct pipe_context *context,
>> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
>> index 9915957..d01deea 100644
>> --- a/src/gallium/include/pipe/p_defines.h
>> +++ b/src/gallium/include/pipe/p_defines.h
>> @@ -458,6 +458,7 @@ enum pipe_flush_flags
>>  #define PIPE_RESOURCE_FLAG_MAP_PERSISTENT (1 << 0)
>>  #define PIPE_RESOURCE_FLAG_MAP_COHERENT   (1 << 1)
>>  #define PIPE_RESOURCE_FLAG_TEXTURING_MORE_LIKELY (1 << 2)
>> +#define PIPE_RESOURCE_FLAG_SPARSE                (1 << 3)
>>  #define PIPE_RESOURCE_FLAG_DRV_PRIV    (1 << 16) /* driver/winsys private */
>>  #define PIPE_RESOURCE_FLAG_ST_PRIV     (1 << 24) /* state-tracker/winsys private */
>>
>> @@ -754,6 +755,7 @@ enum pipe_cap
>>     PIPE_CAP_TGSI_MUL_ZERO_WINS,
>>     PIPE_CAP_DOUBLES,
>>     PIPE_CAP_INT64,
>> +   PIPE_CAP_SPARSE_BUFFER_PAGE_SIZE,
>>  };
>>
>>  #define PIPE_QUIRK_TEXTURE_BORDER_COLOR_SWIZZLE_NV50 (1 << 0)
>> diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
>> index b6203f1..ee0b041 100644
>> --- a/src/gallium/include/pipe/p_screen.h
>> +++ b/src/gallium/include/pipe/p_screen.h
>> @@ -236,7 +236,6 @@ struct pipe_screen {
>>     void (*resource_destroy)(struct pipe_screen *,
>>  			    struct pipe_resource *pt);
>>
>> -
>>     /**
>>      * Do any special operations to ensure frontbuffer contents are
>>      * displayed, eg copy fake frontbuffer.
>>
>