[Mesa-dev] [PATCH 2/7] intel: Remove the pbo zero-copy code.
Eric Anholt
eric at anholt.net
Wed Sep 21 10:15:44 PDT 2011
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.
---
src/mesa/drivers/dri/intel/intel_buffer_objects.c | 45 --------
src/mesa/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, 0 insertions(+), 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 d35a50e..4df2d76 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 d75cdbf..b174e93 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 4c4945c..4d4ddd9 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 91f7121..f3f6a07 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 16a0a24..6db27bc 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)) {
--
1.7.5.4
More information about the mesa-dev
mailing list