On Mon, Jan 30, 2017 at 6:00 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote: > On 27.01.2017 12:02, Marek Olšák wrote: >> >> From: Marek Olšák <marek.ol...@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. Marek _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev