On Tuesday, March 04, 2014 05:34:44 PM Jon Ashburn wrote: > Add Intel driver hook for glGetTexImage to accelerate the case of reading > texture image into a PBO. This case gets huge performance gains by using > GPU BLIT directly to PBO rather than GPU BLIT to temporary texture followed > by memcpy. > > No regressions on Piglit tests with Intel driver. > Performance gain (1280 x 800 FBO, Ivybridge): > glGetTexImage + glMapBufferRange with patch 1.45 msec > glGetTexImage + glMapBufferRange without patch 4.68 msec > --- > src/mesa/drivers/dri/i965/intel_tex_image.c | 105 ++++++++++++++++++++++++++++ > 1 file changed, 105 insertions(+)
I fixed up a bunch of small issues with this patch and pushed it.
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c
b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index ee02e68..13a34d5 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -15,6 +15,8 @@
> #include "main/teximage.h"
> #include "main/texstore.h"
>
> +#include "drivers/common/meta.h"
> +
> #include "intel_mipmap_tree.h"
> #include "intel_buffer_objects.h"
> #include "intel_batchbuffer.h"
> @@ -415,10 +417,113 @@ intel_image_target_texture_2d(struct gl_context *ctx,
GLenum target,
> image->tile_x, image->tile_y);
> }
>
> +static bool
> +IntelBlitTexToPbo(struct gl_context * ctx,
We've been trying to move away from camelCase, and never used StudlyCaps.
I've renamed this to "blit_texture_to_pbo" (we don't need the intel prefix on
static functions).
> + GLenum format, GLenum type,
> + GLvoid * pixels, struct gl_texture_image *texImage)
> +{
> + struct intel_texture_image *intelImage = intel_texture_image(texImage);
> + struct brw_context *brw = brw_context(ctx);
> + const struct gl_pixelstore_attrib *pack = &(ctx->Pack);
> + struct intel_buffer_object *dst = intel_buffer_object(pack->BufferObj);
> + GLuint dst_offset;
> + drm_intel_bo *dst_buffer;
> + GLenum target = texImage->TexObject->Target;
> +
> + /*
Comments should start directly after /*. Fixed.
> + * Check if we can use GPU blit to copy from the hardware texture
> + * format to the user's format/type.
> + * Note that GL's pixel transfer ops don't apply to glGetTexImage()
> + */
> +
> + if (!_mesa_format_matches_format_and_type(
> + intelImage->mt->format, format, type, false))
I reformatted this slightly.
> + {
> + perf_debug("%s: unsupported format, fallback to CPU mapping for
PBO\n", __FUNCTION__);
Wrap at 80 columns. Fixed.
> +
> + return false;
> + }
> +
> + if (ctx->_ImageTransferState) {
> + perf_debug("%s: bad transfer state, fallback to CPU mapping for
PBO\n", __FUNCTION__);
> + return false;
> + }
> +
> + if (pack->SwapBytes || pack->LsbFirst) {
> + perf_debug("%s: unsupported pack swap params\n", __FUNCTION__);
> + return false;
> + }
> +
> + if (target == GL_TEXTURE_1D_ARRAY || target == GL_TEXTURE_CUBE_MAP_ARRAY
||
> + target == GL_TEXTURE_2D_ARRAY) {
This path regresses Piglit's "getteximage-targets 3D" test, which would suffer
from the same "multiple slices" problem. (I see you worked on that test -
perhaps the 3D case didn't exist when you tested this patch?) I blacklisted
GL_TEXTURE_3D here to avoid that bug.
> + perf_debug("%s: no support for array textures, fallback to CPU
mapping for PBO\n", __FUNCTION__);
> + return false;
> + }
> +
> + int dst_stride = _mesa_image_row_stride(pack, texImage->Width, format,
type);
> + bool dst_flip = false;
> + /* Mesa flips the dst_stride for ctx->Pack.Invert, our mt must have a
> + * normal dst_stride.
> + */
> + struct gl_pixelstore_attrib uninverted_pack = *pack;
> + if (ctx->Pack.Invert) {
> + dst_stride = -dst_stride;
> + dst_flip = true;
> + uninverted_pack.Invert = false;
> + }
> + dst_offset = (GLintptr) pixels;
> + dst_offset += _mesa_image_offset(2, &uninverted_pack, texImage->Width,
> + texImage->Height, format, type, 0, 0,
0);
> + dst_buffer = intel_bufferobj_buffer(brw, dst, dst_offset,
> + texImage->Height * dst_stride);
> +
> + struct intel_mipmap_tree *pbo_mt =
> + intel_miptree_create_for_bo(brw,
Weird whitespace. Fixed.
> + dst_buffer,
> + intelImage->mt->format,
> + dst_offset,
> + texImage->Width, texImage->Height,
> + dst_stride, I915_TILING_NONE);
Eric removed the tiling parameter in April, so this didn't compile. Fixed.
Thanks for the patch. Sorry for not getting to review it earlier...
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
