[Mesa-dev] [PATCH] llvmpipe: decrease MAX_SCENES from 2 to 1
From: Roland Scheidegger Multiple scenes per context are meant to be used so a new scene can be built while another one is processed in rasterization. However, quite surprisingly, this does not actually work (and according to git log, possibly never did, though maybe it did at some point further back (5 years+) but was buggy) because we always wait immediately on the rasterizer to finish the scene when contexts (and hence setup/scene) is flushed. This means when we try to get an empty scene later, any old one is already empty again. Thus using multiple scenes is just a waste of memory (not too bad, since the additional scenes are guaranteed to be empty, which means their size ought to be one data block (64kB) plus the size of some structs), without actually really doing anything. (There is also quite some code for the whole concept of multiple scenes which doesn't really do much in practice, but keep it hoping the wait-on-scene-flush can be fixed some day.) --- src/gallium/drivers/llvmpipe/lp_setup.c | 11 +++ src/gallium/drivers/llvmpipe/lp_setup_context.h | 3 ++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c index 7d0c8cc..3b0056c 100644 --- a/src/gallium/drivers/llvmpipe/lp_setup.c +++ b/src/gallium/drivers/llvmpipe/lp_setup.c @@ -165,6 +165,17 @@ lp_setup_rasterize_scene( struct lp_setup_context *setup ) setup->last_fence->issued = TRUE; pipe_mutex_lock(screen->rast_mutex); + + /* FIXME: We enqueue the scene then wait on the rasterizer to finish. +* This means we never actually run any vertex stuff in parallel to +* rasterization (not in the same context at least) which is what the +* multiple scenes per setup is about - when we get a new empty scene +* any old one is already empty again because we waited here for +* raster tasks to be finished. Ideally, we shouldn't need to wait here +* and rely on fences elsewhere when waiting is necessary. +* Certainly, lp_scene_end_rasterization() would need to be deferred too +* and there's probably other bits why this doesn't actually work. +*/ lp_rast_queue_scene(screen->rast, scene); lp_rast_finish(screen->rast); pipe_mutex_unlock(screen->rast_mutex); diff --git a/src/gallium/drivers/llvmpipe/lp_setup_context.h b/src/gallium/drivers/llvmpipe/lp_setup_context.h index ef54403..2410e23 100644 --- a/src/gallium/drivers/llvmpipe/lp_setup_context.h +++ b/src/gallium/drivers/llvmpipe/lp_setup_context.h @@ -55,7 +55,8 @@ struct lp_setup_variant; /** Max number of scenes */ -#define MAX_SCENES 2 +/* XXX: make multiple scenes per context work, see lp_setup_rasterize_scene */ +#define MAX_SCENES 1 -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] llvmpipe: decrease MAX_SCENES from 2 to 1
On 03/12/14 02:59, srol...@vmware.com wrote: From: Roland Scheidegger Multiple scenes per context are meant to be used so a new scene can be built while another one is processed in rasterization. However, quite surprisingly, this does not actually work (and according to git log, possibly never did, though maybe it did at some point further back (5 years+) but was buggy) because we always wait immediately on the rasterizer to finish the scene when contexts (and hence setup/scene) is flushed. This means when we try to get an empty scene later, any old one is already empty again. Thus using multiple scenes is just a waste of memory (not too bad, since the additional scenes are guaranteed to be empty, which means their size ought to be one data block (64kB) plus the size of some structs), without actually really doing anything. (There is also quite some code for the whole concept of multiple scenes which doesn't really do much in practice, but keep it hoping the wait-on-scene-flush can be fixed some day.) --- src/gallium/drivers/llvmpipe/lp_setup.c | 11 +++ src/gallium/drivers/llvmpipe/lp_setup_context.h | 3 ++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c index 7d0c8cc..3b0056c 100644 --- a/src/gallium/drivers/llvmpipe/lp_setup.c +++ b/src/gallium/drivers/llvmpipe/lp_setup.c @@ -165,6 +165,17 @@ lp_setup_rasterize_scene( struct lp_setup_context *setup ) setup->last_fence->issued = TRUE; pipe_mutex_lock(screen->rast_mutex); + + /* FIXME: We enqueue the scene then wait on the rasterizer to finish. +* This means we never actually run any vertex stuff in parallel to +* rasterization (not in the same context at least) which is what the +* multiple scenes per setup is about - when we get a new empty scene +* any old one is already empty again because we waited here for +* raster tasks to be finished. Ideally, we shouldn't need to wait here +* and rely on fences elsewhere when waiting is necessary. +* Certainly, lp_scene_end_rasterization() would need to be deferred too +* and there's probably other bits why this doesn't actually work. +*/ lp_rast_queue_scene(screen->rast, scene); lp_rast_finish(screen->rast); pipe_mutex_unlock(screen->rast_mutex); diff --git a/src/gallium/drivers/llvmpipe/lp_setup_context.h b/src/gallium/drivers/llvmpipe/lp_setup_context.h index ef54403..2410e23 100644 --- a/src/gallium/drivers/llvmpipe/lp_setup_context.h +++ b/src/gallium/drivers/llvmpipe/lp_setup_context.h @@ -55,7 +55,8 @@ struct lp_setup_variant; /** Max number of scenes */ -#define MAX_SCENES 2 +/* XXX: make multiple scenes per context work, see lp_setup_rasterize_scene */ +#define MAX_SCENES 1 Seems sensible AFAICT. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev