From bc0335fc0e0bd6a777ef16ad5245d35ccf7adcf6 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 21 Sep 2011 09:10:19 -0700 Subject: [PATCH] intel: Remove the pbo zero-copy code. There were notes about the possibility of slowdowns due to zcopy from a PBO due to thrashing around of the region. Slowdowns are even more likely now that textures are generally tiled, which a zcopy wouldn't get. Additionally, there were no checks on the buffer size to ensure that the hardware-required rounding was present, which could result in GPU hangs on large zcopy PBOs. Reviewed-by: Ian Romanick --- .../drivers/dri/intel/intel_buffer_objects.c | 45 ------- .../drivers/dri/intel/intel_buffer_objects.h | 12 -- src/mesa/drivers/dri/intel/intel_regions.c | 119 ------------------ src/mesa/drivers/dri/intel/intel_regions.h | 11 -- src/mesa/drivers/dri/intel/intel_tex_image.c | 60 --------- 5 files changed, 247 deletions(-) diff --git a/src/mesa/drivers/dri/intel/intel_buffer_objects.c b/src/mesa/drivers/dri/intel/intel_buffer_objects.c index d35a50ef8b2..4df2d766edb 100644 --- a/src/mesa/drivers/dri/intel/intel_buffer_objects.c +++ b/src/mesa/drivers/dri/intel/intel_buffer_objects.c @@ -79,30 +79,6 @@ intel_bufferobj_alloc(struct gl_context * ctx, GLuint name, GLenum target) return &obj->Base; } -/* Break the COW tie to the region. The region gets to keep the data. - */ -void -intel_bufferobj_release_region(struct intel_buffer_object *intel_obj) -{ - assert(intel_obj->region->buffer == intel_obj->buffer); - intel_obj->region->pbo = NULL; - intel_obj->region = NULL; - - release_buffer(intel_obj); -} - -/* Break the COW tie to the region. Both the pbo and the region end - * up with a copy of the data. - */ -void -intel_bufferobj_cow(struct intel_context *intel, - struct intel_buffer_object *intel_obj) -{ - assert(intel_obj->region); - intel_region_cow(intel, intel_obj->region); -} - - /** * Deallocate/free a vertex/pixel buffer object. * Called via glDeleteBuffersARB(). @@ -122,9 +98,6 @@ intel_bufferobj_free(struct gl_context * ctx, struct gl_buffer_object *obj) intel_bufferobj_unmap(ctx, obj); free(intel_obj->sys_buffer); - if (intel_obj->region) { - intel_bufferobj_release_region(intel_obj); - } drm_intel_bo_unreference(intel_obj->buffer); free(intel_obj); @@ -160,9 +133,6 @@ intel_bufferobj_data(struct gl_context * ctx, assert(!obj->Pointer); /* Mesa should have unmapped it */ - if (intel_obj->region) - intel_bufferobj_release_region(intel_obj); - if (intel_obj->buffer != NULL) release_buffer(intel_obj); @@ -219,9 +189,6 @@ intel_bufferobj_subdata(struct gl_context * ctx, assert(intel_obj); - if (intel_obj->region) - intel_bufferobj_cow(intel, intel_obj); - /* If we have a single copy in system memory, update that */ if (intel_obj->sys_buffer) { if (intel_obj->source) @@ -347,9 +314,6 @@ intel_bufferobj_map_range(struct gl_context * ctx, intel_obj->sys_buffer = NULL; } - if (intel_obj->region) - intel_bufferobj_cow(intel, intel_obj); - /* If the mapping is synchronized with other GL operations, flush * the batchbuffer so that GEM knows about the buffer access for later * syncing. @@ -510,15 +474,6 @@ intel_bufferobj_buffer(struct intel_context *intel, struct intel_buffer_object *intel_obj, GLuint flag) { - if (intel_obj->region) { - if (flag == INTEL_WRITE_PART) - intel_bufferobj_cow(intel, intel_obj); - else if (flag == INTEL_WRITE_FULL) { - intel_bufferobj_release_region(intel_obj); - intel_bufferobj_alloc_buffer(intel, intel_obj); - } - } - if (intel_obj->source) release_buffer(intel_obj); diff --git a/src/mesa/drivers/dri/intel/intel_buffer_objects.h b/src/mesa/drivers/dri/intel/intel_buffer_objects.h index d75cdbf79c2..b174e93443c 100644 --- a/src/mesa/drivers/dri/intel/intel_buffer_objects.h +++ b/src/mesa/drivers/dri/intel/intel_buffer_objects.h @@ -31,7 +31,6 @@ #include "main/mtypes.h" struct intel_context; -struct intel_region; struct gl_buffer_object; @@ -47,10 +46,6 @@ struct intel_buffer_object /** System memory buffer data, if not using a BO to store the data. */ void *sys_buffer; - struct intel_region *region; /* Is there a zero-copy texture - associated with this (pixel) - buffer object? */ - drm_intel_bo *range_map_bo; void *range_map_buffer; unsigned int range_map_offset; @@ -102,11 +97,4 @@ intel_buffer_object(struct gl_buffer_object *obj) return (struct intel_buffer_object *) obj; } -/* Helpers for zerocopy image uploads. See also intel_regions.h: - */ -void intel_bufferobj_cow(struct intel_context *intel, - struct intel_buffer_object *intel_obj); -void intel_bufferobj_release_region(struct intel_buffer_object *intel_obj); - - #endif diff --git a/src/mesa/drivers/dri/intel/intel_regions.c b/src/mesa/drivers/dri/intel/intel_regions.c index 4c4945c7941..4d4ddd9798d 100644 --- a/src/mesa/drivers/dri/intel/intel_regions.c +++ b/src/mesa/drivers/dri/intel/intel_regions.c @@ -115,9 +115,6 @@ intel_region_map(struct intel_context *intel, struct intel_region *region) _DBG("%s %p\n", __FUNCTION__, region); if (!region->map_refcount++) { - if (region->pbo) - intel_region_cow(intel, region); - if (region->tiling != I915_TILING_NONE) drm_intel_gem_bo_map_gtt(region->buffer); else @@ -295,9 +292,6 @@ intel_region_release(struct intel_region **region_handle) if (region->refcount == 0) { assert(region->map_refcount == 0); - if (region->pbo) - region->pbo->region = NULL; - region->pbo = NULL; drm_intel_bo_unreference(region->buffer); if (region->name > 0) @@ -364,14 +358,6 @@ intel_region_data(struct intel_context *intel, if (intel == NULL) return; - if (dst->pbo) { - if (dstx == 0 && - dsty == 0 && width == dst->pitch && height == dst->height) - intel_region_release_pbo(intel, dst); - else - intel_region_cow(intel, dst); - } - intel_prepare_render(intel); _mesa_copy_rect(intel_region_map(intel, dst) + dst_offset, @@ -403,14 +389,6 @@ intel_region_copy(struct intel_context *intel, if (intel == NULL) return GL_FALSE; - if (dst->pbo) { - if (dstx == 0 && - dsty == 0 && width == dst->pitch && height == dst->height) - intel_region_release_pbo(intel, dst); - else - intel_region_cow(intel, dst); - } - assert(src->cpp == dst->cpp); if (flip) @@ -424,106 +402,9 @@ intel_region_copy(struct intel_context *intel, logicop); } -/* Attach to a pbo, discarding our data. Effectively zero-copy upload - * the pbo's data. - */ -void -intel_region_attach_pbo(struct intel_context *intel, - struct intel_region *region, - struct intel_buffer_object *pbo) -{ - drm_intel_bo *buffer; - - if (region->pbo == pbo) - return; - - _DBG("%s %p %p\n", __FUNCTION__, region, pbo); - - /* If there is already a pbo attached, break the cow tie now. - * Don't call intel_region_release_pbo() as that would - * unnecessarily allocate a new buffer we would have to immediately - * discard. - */ - if (region->pbo) { - region->pbo->region = NULL; - region->pbo = NULL; - } - - if (region->buffer) { - drm_intel_bo_unreference(region->buffer); - region->buffer = NULL; - } - - /* make sure pbo has a buffer of its own */ - buffer = intel_bufferobj_buffer(intel, pbo, INTEL_WRITE_FULL); - - region->pbo = pbo; - region->pbo->region = region; - drm_intel_bo_reference(buffer); - region->buffer = buffer; - region->tiling = I915_TILING_NONE; -} - - -/* Break the COW tie to the pbo and allocate a new buffer. - * The pbo gets to keep the data. - */ -void -intel_region_release_pbo(struct intel_context *intel, - struct intel_region *region) -{ - _DBG("%s %p\n", __FUNCTION__, region); - assert(region->buffer == region->pbo->buffer); - region->pbo->region = NULL; - region->pbo = NULL; - drm_intel_bo_unreference(region->buffer); - region->buffer = NULL; - - region->buffer = drm_intel_bo_alloc(intel->bufmgr, "region", - region->pitch * region->cpp * - region->height, - 64); -} - -/* Break the COW tie to the pbo. Both the pbo and the region end up - * with a copy of the data. - */ -void -intel_region_cow(struct intel_context *intel, struct intel_region *region) -{ - struct intel_buffer_object *pbo = region->pbo; - GLboolean ok; - - intel_region_release_pbo(intel, region); - - assert(region->cpp * region->pitch * region->height == pbo->Base.Size); - - _DBG("%s %p (%d bytes)\n", __FUNCTION__, region, (int)pbo->Base.Size); - - /* Now blit from the texture buffer to the new buffer: - */ - - intel_prepare_render(intel); - ok = intelEmitCopyBlit(intel, - region->cpp, - region->pitch, pbo->buffer, 0, region->tiling, - region->pitch, region->buffer, 0, region->tiling, - 0, 0, 0, 0, - region->pitch, region->height, - GL_COPY); - assert(ok); -} - drm_intel_bo * intel_region_buffer(struct intel_context *intel, struct intel_region *region, GLuint flag) { - if (region->pbo) { - if (flag == INTEL_WRITE_PART) - intel_region_cow(intel, region); - else if (flag == INTEL_WRITE_FULL) - intel_region_release_pbo(intel, region); - } - return region->buffer; } diff --git a/src/mesa/drivers/dri/intel/intel_regions.h b/src/mesa/drivers/dri/intel/intel_regions.h index 91f7121436e..f3f6a07901c 100644 --- a/src/mesa/drivers/dri/intel/intel_regions.h +++ b/src/mesa/drivers/dri/intel/intel_regions.h @@ -63,7 +63,6 @@ struct intel_region GLuint map_refcount; /**< Reference count for mapping */ uint32_t tiling; /**< Which tiling mode the region is in */ - struct intel_buffer_object *pbo; /* zero-copy uploads */ uint32_t name; /**< Global name for the bo */ struct intel_screen *screen; @@ -125,16 +124,6 @@ intel_region_copy(struct intel_context *intel, GLboolean flip, GLenum logicop); -/* Helpers for zerocopy uploads, particularly texture image uploads: - */ -void intel_region_attach_pbo(struct intel_context *intel, - struct intel_region *region, - struct intel_buffer_object *pbo); -void intel_region_release_pbo(struct intel_context *intel, - struct intel_region *region); -void intel_region_cow(struct intel_context *intel, - struct intel_region *region); - drm_intel_bo *intel_region_buffer(struct intel_context *intel, struct intel_region *region, GLuint flag); diff --git a/src/mesa/drivers/dri/intel/intel_tex_image.c b/src/mesa/drivers/dri/intel/intel_tex_image.c index 16a0a2492d4..6db27bca851 100644 --- a/src/mesa/drivers/dri/intel/intel_tex_image.c +++ b/src/mesa/drivers/dri/intel/intel_tex_image.c @@ -213,49 +213,6 @@ try_pbo_upload(struct intel_context *intel, return GL_TRUE; } - -static GLboolean -try_pbo_zcopy(struct intel_context *intel, - struct intel_texture_image *intelImage, - const struct gl_pixelstore_attrib *unpack, - GLint width, const void *pixels) -{ - struct intel_buffer_object *pbo = intel_buffer_object(unpack->BufferObj); - GLuint src_offset, src_stride; - GLuint dst_x, dst_y, dst_stride; - - if (!_mesa_is_bufferobj(unpack->BufferObj) || - intel->ctx._ImageTransferState || - unpack->SkipPixels || unpack->SkipRows) { - DBG("%s: failure 1\n", __FUNCTION__); - return GL_FALSE; - } - - /* note: potential 64-bit ptr to 32-bit int cast */ - src_offset = (GLuint) (unsigned long) pixels; - - if (unpack->RowLength > 0) - src_stride = unpack->RowLength; - else - src_stride = width; - - intel_miptree_get_image_offset(intelImage->mt, intelImage->base.Base.Level, - intelImage->base.Base.Face, 0, - &dst_x, &dst_y); - - dst_stride = intelImage->mt->region->pitch; - - if (src_stride != dst_stride || dst_x != 0 || dst_y != 0 || - src_offset != 0) { - DBG("%s: failure 2\n", __FUNCTION__); - return GL_FALSE; - } - - intel_region_attach_pbo(intel, intelImage->mt->region, pbo); - - return GL_TRUE; -} - /** * \param scatter Scatter if true. Gather if false. * @@ -451,23 +408,6 @@ intelTexImage(struct gl_context * ctx, DBG("trying pbo upload\n"); - /* Attempt to texture directly from PBO data (zero copy upload). - * - * Currently disable as it can lead to worse as well as better - * performance (in particular when intel_region_cow() is - * required). - */ - if (intelObj->mt == intelImage->mt && - intelObj->mt->first_level == level && - intelObj->mt->last_level == level) { - - if (try_pbo_zcopy(intel, intelImage, unpack, width, pixels)) { - DBG("pbo zcopy upload succeeded\n"); - return; - } - } - - /* Otherwise, attempt to use the blitter for PBO image uploads. */ if (try_pbo_upload(intel, intelImage, unpack, width, height, pixels)) { -- 2.30.2