[Mesa-dev] [PATCH] llvmpipe: decrease MAX_SCENES from 2 to 1

[Jose Fonseca]

On 03/12/14 02:59, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> 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