[Mesa-dev] [PATCH 2/2] i965: Enhance tiled_memcpy to support all levels

[Chad Versace]

On 11/08/2013 08:13 AM, Courtney Goeltzenleuchter wrote:
> Support all levels of a supported texture format.
>
> Using 1024x1024, RGBA 8888 source, mipmap
> 					            <<THIS PATCH>>
> internal-format	Before (MB/sec)     XRGB (MB/sec)	mipmap (MB/sec)
> GL_RGBA		628.15		    627.15		615.90
> GL_RGB		265.95	 	    456.35		611.53
> 512x512
> GL_RGBA		600.23	  	    597.00		619.95
> GL_RGB		255.50		    440.62		611.28
> 256x256
> GL_RGBA		489.08		    487.80		587.42
> GL_RGB		229.03	  	    376.63		585.00
>
> Test shows similar pattern for 512x512 and 256x256.
>
> Benchmark has been sent to mesa-dev list: teximage_enh
>
> Courtney Goeltzenleuchter (2):
>    i965: add XRGB to tiled_memcpy
>    i965: Enhance tiled_memcpy to support all levels
>
>   src/mesa/drivers/dri/i965/intel_tex_subimage.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> --
> 1.8.1.2
>
> Signed-off-by: Courtney Goeltzenleuchter <courtney at LunarG.com>
> ---
>   src/mesa/drivers/dri/i965/intel_tex_subimage.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> index b1826fa..50f802f 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> @@ -543,7 +543,7 @@ intel_texsubimage_tiled_memcpy(struct gl_context * ctx,
>      uint32_t cpp;
>      mem_copy_fn mem_copy = NULL;
>
> -   /* This fastpath is restricted to specific texture types: level 0 of
> +   /* This fastpath is restricted to specific texture types:
>       * a 2D BGRA, RGBA, L8 or A8 texture. It could be generalized to support
>       * more types.
>       *
> @@ -555,7 +555,6 @@ intel_texsubimage_tiled_memcpy(struct gl_context * ctx,
>      if (!brw->has_llc ||
>          type != GL_UNSIGNED_BYTE ||
>          texImage->TexObject->Target != GL_TEXTURE_2D ||
> -       texImage->Level != 0 ||
>          pixels == NULL ||
>          _mesa_is_bufferobj(packing->BufferObj) ||
>          packing->Alignment > 4 ||
> @@ -631,6 +630,11 @@ intel_texsubimage_tiled_memcpy(struct gl_context * ctx,
>          packing->Alignment, packing->RowLength, packing->SkipPixels,
>          packing->SkipRows, for_glTexImage);
>
> +   /* Adjust x and y offset based on miplevel
> +    */

One small nitpick. The above comment is short enough to fit on a single line.
There's no need to place '*/' on a line by itself.

> +   xoffset += image->mt->level[texImage->Level].level_x;
> +   yoffset += image->mt->level[texImage->Level].level_y;
> +
>      linear_to_tiled(
>         xoffset * cpp, (xoffset + width) * cpp,
>         yoffset, yoffset + height,
>

The code looks good, though I haven't tested it yet. But, I see the same
issues with the commit message that I had with patch 1.