[Mesa-dev] [PATCH] llvmpipe: Stop calling util_format_get_blocksize on the hot path

[Roland Scheidegger]

Am 10.06.2013 22:34, schrieb Adam Jackson:
> Calling this two+ times per tile command is not especially fast.  We
> already had a slot for this in the scene, but we were only filling it in
> for the z/s buffer.  Fill it in consistently and use that instead of
> util_format_get_blocksize() when computing tile addresses.
> 
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
>  src/gallium/drivers/llvmpipe/lp_rast_priv.h | 8 ++++----
>  src/gallium/drivers/llvmpipe/lp_scene.c     | 2 ++
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast_priv.h b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> index 6f03023..7951328 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> +++ b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> @@ -162,7 +162,7 @@ lp_rast_get_unswizzled_color_tile_pointer(struct lp_rasterizer_task *task,
>        struct pipe_surface *cbuf = scene->fb.cbufs[buf];
>        assert(cbuf);
>  
> -      format_bytes = util_format_get_blocksize(cbuf->format);
> +      format_bytes = scene->cbufs[buf].blocksize;
>        task->color_tiles[buf] = scene->cbufs[buf].map + scene->cbufs[buf].stride * task->y + format_bytes * task->x;
>     }
>  
> @@ -189,7 +189,7 @@ lp_rast_get_unswizzled_depth_tile_pointer(struct lp_rasterizer_task *task,
>        struct pipe_surface *dbuf = scene->fb.zsbuf;
>        assert(dbuf);
>  
> -      format_bytes = util_format_get_blocksize(dbuf->format);
> +      format_bytes = scene->zsbuf.blocksize;
>        task->depth_tile = scene->zsbuf.map + scene->zsbuf.stride * task->y + format_bytes * task->x;
>     }
>  
> @@ -215,7 +215,7 @@ lp_rast_get_unswizzled_color_block_pointer(struct lp_rasterizer_task *task,
>     assert((y % TILE_VECTOR_HEIGHT) == 0);
>     assert(buf < task->scene->fb.nr_cbufs);
>  
> -   format_bytes = util_format_get_blocksize(task->scene->fb.cbufs[buf]->format);
> +   format_bytes = task->scene->cbufs[buf].blocksize;
>  
>     color = lp_rast_get_unswizzled_color_tile_pointer(task, buf, LP_TEX_USAGE_READ_WRITE);
>     assert(color);
> @@ -251,7 +251,7 @@ lp_rast_get_unswizzled_depth_block_pointer(struct lp_rasterizer_task *task,
>     assert((x % TILE_VECTOR_WIDTH) == 0);
>     assert((y % TILE_VECTOR_HEIGHT) == 0);
>  
> -   format_bytes = util_format_get_blocksize(task->scene->fb.zsbuf->format);
> +   format_bytes = task->scene->zsbuf.blocksize;
>  
>     depth = lp_rast_get_unswizzled_depth_tile_pointer(task, LP_TEX_USAGE_READ_WRITE);
>     assert(depth);
> diff --git a/src/gallium/drivers/llvmpipe/lp_scene.c b/src/gallium/drivers/llvmpipe/lp_scene.c
> index 2abbd25..d1cb791 100644
> --- a/src/gallium/drivers/llvmpipe/lp_scene.c
> +++ b/src/gallium/drivers/llvmpipe/lp_scene.c
> @@ -168,6 +168,7 @@ lp_scene_begin_rasterization(struct lp_scene *scene)
>                                                       cbuf->u.tex.level,
>                                                       cbuf->u.tex.first_layer,
>                                                       LP_TEX_USAGE_READ_WRITE);
> +         scene->cbufs[i].blocksize = util_format_get_blocksize(cbuf->format);
>        }
>        else {
>           struct llvmpipe_resource *lpr = llvmpipe_resource(cbuf->texture);
> @@ -177,6 +178,7 @@ lp_scene_begin_rasterization(struct lp_scene *scene)
>  
>           scene->cbufs[i].map = lpr->data;
>           scene->cbufs[i].map += cbuf->u.buf.first_element * pixstride;
> +         scene->cbufs[i].blocksize = pixstride;
>        }
>     }
>  
> 

The idea looks good to me but it won't compile, as I've removed the
blocksize var recently (d8146f240e628e70d4c07f7e805a179f70c36e23).
Exactly because it was used in only one lonely place (for clearing
depth, was not even assigned for color buffers), so I figured there's
two options, just get rid of it or use it consistently, and I haven't
really seen this show up a lot in profiles. I didn't do all that much
profiling, so if it indeed shows up I'm all for it. (I think it was
always used when we had swizzled rendering, and it got unused for color
render targets when these switched to linear layout, and also for the
linear layout depth path later, except for that one use when clearing
depth.)
So if you restore these bits this looks good to me.

Roland