[Mesa-dev] [PATCH 01/10] gallium: add a common uploader to pipe_context

[Marek Olšák]

On Fri, Jan 27, 2017 at 3:38 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 27.01.2017 um 12:02 schrieb Marek Olšák:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> For lower memory usage and more efficient updates of the buffer residency
>> list. (e.g. if drivers keep seeing the same buffer for many consecutive
>> "add" calls, the calls can be turned into no-ops trivially)
>> ---
>>  src/gallium/include/pipe/p_context.h | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
>> index 45098c9..5876968 100644
>> --- a/src/gallium/include/pipe/p_context.h
>> +++ b/src/gallium/include/pipe/p_context.h
>> @@ -69,33 +69,40 @@ struct pipe_stream_output_target;
>>  struct pipe_surface;
>>  struct pipe_transfer;
>>  struct pipe_vertex_buffer;
>>  struct pipe_vertex_element;
>>  struct pipe_video_buffer;
>>  struct pipe_video_codec;
>>  struct pipe_viewport_state;
>>  struct pipe_compute_state;
>>  union pipe_color_union;
>>  union pipe_query_result;
>> +struct u_upload_mgr;
>>
>>  /**
>>   * Gallium rendering context.  Basically:
>>   *  - state setting functions
>>   *  - VBO drawing functions
>>   *  - surface functions
>>   */
>>  struct pipe_context {
>>     struct pipe_screen *screen;
>>
>>     void *priv;  /**< context private data (for DRI for example) */
>>     void *draw;  /**< private, for draw module (temporary?) */
>>
>> +   /**
>> +    * Stream uploader created by the driver. All drivers, state trackers, and
>> +    * modules should use it.
>> +    */
>> +   struct u_upload_mgr *stream_uploader;
>> +
>>     void (*destroy)( struct pipe_context * );
>>
>>     /**
>>      * VBO drawing
>>      */
>>     /*@{*/
>>     void (*draw_vbo)( struct pipe_context *pipe,
>>                       const struct pipe_draw_info *info );
>>     /*@}*/
>>
>>
>
> I suppose this makes sense. However, this makes util interfaces
> effectively part of the gallium interface, not sure how I feel about
> that as this seems to violate the contract that util code is optional.

To be honest, I don't care much. Putting the uploader into
pipe_context has an obvious practical advantage and we want exactly
one instance between drivers and state trackers.

Also, all utilities that had to use pipe_buffer_create just to upload
something for one use can now use this.

If somebody wants a proper formal interface, feel free.

I do see a small decrease in CPU overhead with legacy GL apps thanks
to amdgpu_cs_add_buffer getting the same buffer over and over again.

Marek