[Mesa-dev] [PATCH 03/10] gallium: set pipe_context::stream_uploader
Nicolai Hähnle
nhaehnle at gmail.com
Tue Jan 31 11:12:25 UTC 2017
On 31.01.2017 02:37, Marek Olšák wrote:
> On Mon, Jan 30, 2017 at 6:29 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> On 30.01.2017 18:20, Marek Olšák wrote:
>>>
>>> On Mon, Jan 30, 2017 at 6:00 PM, Nicolai Hähnle <nhaehnle at gmail.com>
>>> wrote:
>>>>
>>>> On 27.01.2017 12:02, Marek Olšák wrote:
>>>>>
>>>>>
>>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>>
>>>>> Notes:
>>>>> - make sure the default size is large enough to handle all state
>>>>> trackers
>>>>> - pipe wrappers don't receive transfer calls from stream_uploader,
>>>>> because
>>>>> pipe_context::stream_uploader points directly to the underlying
>>>>> driver's
>>>>> stream_uploader (to keep it simple for now)
>>>>> ---
>>>>> src/gallium/drivers/ddebug/dd_context.c | 1 +
>>>>> src/gallium/drivers/etnaviv/etnaviv_context.c | 7 +++++++
>>>>> src/gallium/drivers/freedreno/freedreno_context.c | 8 ++++++++
>>>>> src/gallium/drivers/i915/i915_context.c | 5 +++++
>>>>> src/gallium/drivers/ilo/ilo_context.c | 1 +
>>>>> src/gallium/drivers/llvmpipe/lp_context.c | 8 ++++++++
>>>>> src/gallium/drivers/noop/noop_pipe.c | 5 +++++
>>>>> src/gallium/drivers/nouveau/nv30/nv30_context.c | 10 ++++++++++
>>>>> src/gallium/drivers/nouveau/nv50/nv50_context.c | 6 ++++++
>>>>> src/gallium/drivers/nouveau/nvc0/nvc0_context.c | 5 +++++
>>>>> src/gallium/drivers/r300/r300_context.c | 3 ++-
>>>>> src/gallium/drivers/radeon/r600_pipe_common.c | 1 +
>>>>> src/gallium/drivers/rbug/rbug_context.c | 1 +
>>>>> src/gallium/drivers/softpipe/sp_context.c | 7 +++++++
>>>>> src/gallium/drivers/svga/svga_context.c | 6 ++++++
>>>>> src/gallium/drivers/swr/swr_context.cpp | 8 ++++++++
>>>>> src/gallium/drivers/trace/tr_context.c | 1 +
>>>>> src/gallium/drivers/vc4/vc4_context.c | 10 +++++-----
>>>>> src/gallium/drivers/virgl/virgl_context.c | 1 +
>>>>> 19 files changed, 88 insertions(+), 6 deletions(-)
>>>>>
>>>> [snip]
>>>>
>>>>> diff --git a/src/gallium/drivers/noop/noop_pipe.c
>>>>> b/src/gallium/drivers/noop/noop_pipe.c
>>>>> index 3013019..6ef4f6f 100644
>>>>> --- a/src/gallium/drivers/noop/noop_pipe.c
>>>>> +++ b/src/gallium/drivers/noop/noop_pipe.c
>>>>> @@ -22,20 +22,21 @@
>>>>> */
>>>>> #include <stdio.h>
>>>>> #include <errno.h>
>>>>> #include "pipe/p_defines.h"
>>>>> #include "pipe/p_state.h"
>>>>> #include "pipe/p_context.h"
>>>>> #include "pipe/p_screen.h"
>>>>> #include "util/u_memory.h"
>>>>> #include "util/u_inlines.h"
>>>>> #include "util/u_format.h"
>>>>> +#include "util/u_upload_mgr.h"
>>>>> #include "noop_public.h"
>>>>>
>>>>> DEBUG_GET_ONCE_BOOL_OPTION(noop, "GALLIUM_NOOP", FALSE)
>>>>>
>>>>> void noop_init_state_functions(struct pipe_context *ctx);
>>>>>
>>>>> struct noop_pipe_screen {
>>>>> struct pipe_screen pscreen;
>>>>> struct pipe_screen *oscreen;
>>>>> };
>>>>> @@ -282,20 +283,23 @@ noop_flush_resource(struct pipe_context *ctx,
>>>>> static void noop_flush(struct pipe_context *ctx,
>>>>> struct pipe_fence_handle **fence,
>>>>> unsigned flags)
>>>>> {
>>>>> if (fence)
>>>>> *fence = NULL;
>>>>> }
>>>>>
>>>>> static void noop_destroy_context(struct pipe_context *ctx)
>>>>> {
>>>>> + if (ctx->stream_uploader)
>>>>> + u_upload_destroy(ctx->stream_uploader);
>>>>> +
>>>>> FREE(ctx);
>>>>> }
>>>>>
>>>>> static boolean noop_generate_mipmap(struct pipe_context *ctx,
>>>>> struct pipe_resource *resource,
>>>>> enum pipe_format format,
>>>>> unsigned base_level,
>>>>> unsigned last_level,
>>>>> unsigned first_layer,
>>>>> unsigned last_layer)
>>>>> @@ -305,20 +309,21 @@ static boolean noop_generate_mipmap(struct
>>>>> pipe_context *ctx,
>>>>>
>>>>> static struct pipe_context *noop_create_context(struct pipe_screen
>>>>> *screen,
>>>>> void *priv, unsigned
>>>>> flags)
>>>>> {
>>>>> struct pipe_context *ctx = CALLOC_STRUCT(pipe_context);
>>>>>
>>>>> if (!ctx)
>>>>> return NULL;
>>>>> ctx->screen = screen;
>>>>> ctx->priv = priv;
>>>>> + ctx->stream_uploader = u_upload_create_default(ctx);
>>>>
>>>>
>>>>
>>>> Error handling would be nice. (The allocation of the u_upload_mgr struct
>>>> itself can still fail.)
>>>>
>>>>
>>>>> ctx->destroy = noop_destroy_context;
>>>>> ctx->flush = noop_flush;
>>>>> ctx->clear = noop_clear;
>>>>> ctx->clear_render_target = noop_clear_render_target;
>>>>> ctx->clear_depth_stencil = noop_clear_depth_stencil;
>>>>> ctx->resource_copy_region = noop_resource_copy_region;
>>>>> ctx->generate_mipmap = noop_generate_mipmap;
>>>>> ctx->blit = noop_blit;
>>>>> ctx->flush_resource = noop_flush_resource;
>>>>> ctx->create_query = noop_create_query;
>>>>
>>>>
>>>> [snip]
>>>>
>>>>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c
>>>>> b/src/gallium/drivers/nouveau/nv50/nv50_context.c
>>>>> index ece7da9..9d34c4d 100644
>>>>> --- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
>>>>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
>>>>> @@ -15,20 +15,21 @@
>>>>> * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>>>> MERCHANTABILITY,
>>>>> * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>>>>> SHALL
>>>>> * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>>>> * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>>>>> OTHERWISE,
>>>>> * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
>>>>> OR
>>>>> * OTHER DEALINGS IN THE SOFTWARE.
>>>>> */
>>>>>
>>>>> #include "pipe/p_defines.h"
>>>>> #include "util/u_framebuffer.h"
>>>>> +#include "util/u_upload_mgr.h"
>>>>>
>>>>> #include "nv50/nv50_context.h"
>>>>> #include "nv50/nv50_screen.h"
>>>>> #include "nv50/nv50_resource.h"
>>>>>
>>>>> static void
>>>>> nv50_flush(struct pipe_context *pipe,
>>>>> struct pipe_fence_handle **fence,
>>>>> unsigned flags)
>>>>> {
>>>>> @@ -169,20 +170,24 @@ nv50_context_unreference_resources(struct
>>>>> nv50_context *nv50)
>>>>> static void
>>>>> nv50_destroy(struct pipe_context *pipe)
>>>>> {
>>>>> struct nv50_context *nv50 = nv50_context(pipe);
>>>>>
>>>>> if (nv50->screen->cur_ctx == nv50) {
>>>>> nv50->screen->cur_ctx = NULL;
>>>>> /* Save off the state in case another context gets created */
>>>>> nv50->screen->save_state = nv50->state;
>>>>> }
>>>>> +
>>>>> + if (nv50->base.pipe.stream_uploader)
>>>>> + u_upload_destroy(nv50->base.pipe.stream_uploader);
>>>>> +
>>>>> nouveau_pushbuf_bufctx(nv50->base.pushbuf, NULL);
>>>>> nouveau_pushbuf_kick(nv50->base.pushbuf,
>>>>> nv50->base.pushbuf->channel);
>>>>>
>>>>> nv50_context_unreference_resources(nv50);
>>>>>
>>>>> FREE(nv50->blit);
>>>>>
>>>>> nouveau_context_destroy(&nv50->base);
>>>>> }
>>>>>
>>>>> @@ -308,20 +313,21 @@ nv50_create(struct pipe_screen *pscreen, void
>>>>> *priv,
>>>>> unsigned ctxflags)
>>>>> goto out_err;
>>>>>
>>>>> nv50->base.screen = &screen->base;
>>>>> nv50->base.copy_data = nv50_m2mf_copy_linear;
>>>>> nv50->base.push_data = nv50_sifc_linear_u8;
>>>>> nv50->base.push_cb = nv50_cb_push;
>>>>>
>>>>> nv50->screen = screen;
>>>>> pipe->screen = pscreen;
>>>>> pipe->priv = priv;
>>>>> + pipe->stream_uploader = u_upload_create_default(pipe);
>>>>
>>>>
>>>>
>>>> Same here.
>>>>
>>>>
>>>>
>>>>>
>>>>> pipe->destroy = nv50_destroy;
>>>>>
>>>>> pipe->draw_vbo = nv50_draw_vbo;
>>>>> pipe->clear = nv50_clear;
>>>>> pipe->launch_grid = nv50_launch_grid;
>>>>>
>>>>> pipe->flush = nv50_flush;
>>>>> pipe->texture_barrier = nv50_texture_barrier;
>>>>> pipe->memory_barrier = nv50_memory_barrier;
>>>>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.c
>>>>> b/src/gallium/drivers/nouveau/nvc0/nvc0_context.c
>>>>> index 8f2b974..5921fa6 100644
>>>>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.c
>>>>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.c
>>>>> @@ -15,20 +15,21 @@
>>>>> * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>>>> MERCHANTABILITY,
>>>>> * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>>>>> SHALL
>>>>> * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>>>> * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>>>>> OTHERWISE,
>>>>> * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
>>>>> OR
>>>>> * OTHER DEALINGS IN THE SOFTWARE.
>>>>> */
>>>>>
>>>>> #include "pipe/p_defines.h"
>>>>> #include "util/u_framebuffer.h"
>>>>> +#include "util/u_upload_mgr.h"
>>>>>
>>>>> #include "nvc0/nvc0_context.h"
>>>>> #include "nvc0/nvc0_screen.h"
>>>>> #include "nvc0/nvc0_resource.h"
>>>>>
>>>>> static void
>>>>> nvc0_flush(struct pipe_context *pipe,
>>>>> struct pipe_fence_handle **fence,
>>>>> unsigned flags)
>>>>> {
>>>>> @@ -192,20 +193,23 @@ static void
>>>>> nvc0_destroy(struct pipe_context *pipe)
>>>>> {
>>>>> struct nvc0_context *nvc0 = nvc0_context(pipe);
>>>>>
>>>>> if (nvc0->screen->cur_ctx == nvc0) {
>>>>> nvc0->screen->cur_ctx = NULL;
>>>>> nvc0->screen->save_state = nvc0->state;
>>>>> nvc0->screen->save_state.tfb = NULL;
>>>>> }
>>>>>
>>>>> + if (nvc0->base.pipe.stream_uploader)
>>>>> + u_upload_destroy(nvc0->base.pipe.stream_uploader);
>>>>> +
>>>>> /* Unset bufctx, we don't want to revalidate any resources after the
>>>>> flush.
>>>>> * Other contexts will always set their bufctx again on action
>>>>> calls.
>>>>> */
>>>>> nouveau_pushbuf_bufctx(nvc0->base.pushbuf, NULL);
>>>>> nouveau_pushbuf_kick(nvc0->base.pushbuf,
>>>>> nvc0->base.pushbuf->channel);
>>>>>
>>>>> nvc0_context_unreference_resources(nvc0);
>>>>> nvc0_blitctx_destroy(nvc0);
>>>>>
>>>>> nouveau_context_destroy(&nvc0->base);
>>>>> @@ -379,20 +383,21 @@ nvc0_create(struct pipe_screen *pscreen, void
>>>>> *priv,
>>>>> unsigned ctxflags)
>>>>> ret = nouveau_bufctx_new(screen->base.client, NVC0_BIND_CP_COUNT,
>>>>> &nvc0->bufctx_cp);
>>>>> if (ret)
>>>>> goto out_err;
>>>>>
>>>>> nvc0->screen = screen;
>>>>> nvc0->base.screen = &screen->base;
>>>>>
>>>>> pipe->screen = pscreen;
>>>>> pipe->priv = priv;
>>>>> + pipe->stream_uploader = u_upload_create_default(pipe);
>>>>
>>>>
>>>>
>>>> And here.
>>>>
>>>>
>>>>>
>>>>> pipe->destroy = nvc0_destroy;
>>>>>
>>>>> pipe->draw_vbo = nvc0_draw_vbo;
>>>>> pipe->clear = nvc0_clear;
>>>>> pipe->launch_grid = (nvc0->screen->base.class_3d >= NVE4_3D_CLASS) ?
>>>>> nve4_launch_grid : nvc0_launch_grid;
>>>>>
>>>>> pipe->flush = nvc0_flush;
>>>>> pipe->texture_barrier = nvc0_texture_barrier;
>>>>> diff --git a/src/gallium/drivers/r300/r300_context.c
>>>>> b/src/gallium/drivers/r300/r300_context.c
>>>>> index b914cdb..eba5ea5 100644
>>>>> --- a/src/gallium/drivers/r300/r300_context.c
>>>>> +++ b/src/gallium/drivers/r300/r300_context.c
>>>>> @@ -417,22 +417,23 @@ struct pipe_context* r300_create_context(struct
>>>>> pipe_screen* screen,
>>>>> r300_init_flush_functions(r300);
>>>>> r300_init_query_functions(r300);
>>>>> r300_init_state_functions(r300);
>>>>> r300_init_resource_functions(r300);
>>>>> r300_init_render_functions(r300);
>>>>> r300_init_states(&r300->context);
>>>>>
>>>>> r300->context.create_video_codec = vl_create_decoder;
>>>>> r300->context.create_video_buffer = vl_video_buffer_create;
>>>>>
>>>>> - r300->uploader = u_upload_create(&r300->context, 256 * 1024,
>>>>> + r300->uploader = u_upload_create(&r300->context, 1024 * 1024,
>>>>> PIPE_BIND_CUSTOM,
>>>>> PIPE_USAGE_STREAM);
>>>>> + r300->context.stream_uploader = r300->uploader;
>>>>
>>>>
>>>>
>>>> ... and here, I guess? Looks like error handling was already missing...
>>>>
>>>>
>>>> [snip]
>>>>>
>>>>>
>>>>> diff --git a/src/gallium/drivers/vc4/vc4_context.c
>>>>> b/src/gallium/drivers/vc4/vc4_context.c
>>>>> index 974df8a..407f4d9 100644
>>>>> --- a/src/gallium/drivers/vc4/vc4_context.c
>>>>> +++ b/src/gallium/drivers/vc4/vc4_context.c
>>>>> @@ -137,33 +137,33 @@ vc4_context_create(struct pipe_screen *pscreen,
>>>>> void
>>>>> *priv, unsigned flags)
>>>>> vc4_state_init(pctx);
>>>>> vc4_program_init(pctx);
>>>>> vc4_query_init(pctx);
>>>>> vc4_resource_context_init(pctx);
>>>>>
>>>>> vc4_job_init(vc4);
>>>>>
>>>>> vc4->fd = screen->fd;
>>>>>
>>>>> slab_create_child(&vc4->transfer_pool, &screen->transfer_pool);
>>>>> - vc4->blitter = util_blitter_create(pctx);
>>>>> +
>>>>> + vc4->uploader = u_upload_create_default(&vc4->base);
>>>>> + vc4->base.stream_uploader = vc4->uploader;
>>>>> +
>>>>> + vc4->blitter = util_blitter_create(pctx);
>>>>
>>>>
>>>>
>>>> Same story as r300.
>>>
>>>
>>> All the missing error handling is due to the fact the drivers don't do
>>> any error handling. I didn't want to invent ways to free the context
>>> in all the drivers that I can't test.
>>
>>
>> The noop case is trivial, and for nv50 and nvc0 I do see a pattern of `goto
>> out_err`s. I see your point for r300 and vc4, though.
>
> Done:
> https://cgit.freedesktop.org/~mareko/mesa/commit/?h=master&id=cf9bda74c291d1e134afd99330e2c8382b0ac8ff
Thanks. The series is good to go as far as I'm concerned.
Cheers,
Nicolai
>
> Marek
>
More information about the mesa-dev
mailing list