FYI, if there are no other comments, I'll push this series.
Marek
On Mon, Jan 30, 2017 at 6:06 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> On 27.01.2017 16:02, Marek Olšák wrote:
>>
>> On Fri, Jan 27, 2017 at 3:38 PM, Roland Scheidegger <srol...@vmware.com>
>> wrote:
>>>
>>> Am 27.01.2017 um 12:02 schrieb Marek Olšák:
>>>>
>>>> From: Marek Olšák <marek.ol...@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.
>
>
> I agree that this is a nice cleanup and improvement at the same time.
>
> Roland has a point, of course, and in theory one could add an upload_alloc
> function to the pipe_context, but in practice, that just adds an indirect
> call for no benefit. I think we should go ahead with this.
>
> I have some comments on patch #3, but apart from that, the series is
>
> Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
>
>>
>> Marek
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
