From: Eric Anholt <eric@anholt.net>
Date: Wed, 21 Sep 2011 16:10:19 +0000 (-0700)
Subject: intel: Remove the pbo zero-copy code.
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=bc0335fc0e0bd6a777ef16ad5245d35ccf7adcf6;p=mesa.git

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 <ian.d.romanick@intel.com>
---

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)) {