On 10/10/2011 11:17 AM, Eric Anholt wrote:
On Thu,  6 Oct 2011 14:18:35 -0700, Chad Versace<c...@chad-versace.us>  wrote:
What I would prefer to assert is that, for each region that is currently
mapped, no batch is emitted that uses that region's bo. However, it's much
easier to implement this big hammer.

diff --git a/src/mesa/drivers/dri/intel/intel_regions.c 
b/src/mesa/drivers/dri/intel/intel_regions.c
index 7faf4ca..d46c470 100644
--- a/src/mesa/drivers/dri/intel/intel_regions.c
+++ b/src/mesa/drivers/dri/intel/intel_regions.c
@@ -111,15 +111,17 @@ debug_backtrace(void)
  GLubyte *
  intel_region_map(struct intel_context *intel, struct intel_region *region)
  {
-   intel_flush(&intel->ctx);
-
     _DBG("%s %p\n", __FUNCTION__, region);
     if (!region->map_refcount++) {
+      intel_flush(&intel->ctx);
+
        if (region->tiling != I915_TILING_NONE)
         drm_intel_gem_bo_map_gtt(region->bo);
        else
         drm_intel_bo_map(region->bo, GL_TRUE);
+
        region->map = region->bo->virtual;
+      ++intel->num_mapped_regions;
     }

There's a more interesting change in here, which is the move of the
flush inside of the if statement.

We agreed that moving it in was correct, but it would be nice to record
why we thought it was correct.  The thinking I recall was:

"We have the region->map_refcount controlling mapping of the BO because
in software fallbacks we may end up mapping the same buffer multiple
times on Mesa's behalf, so we refcount our mappings to make sure that
the pointer stays valid until the end of the unmap chain.  However, we
must not emit any batchbuffers between the start of mapping and the end
of unmapping, or further use of the map will be incoherent with the GPU
rendering done by that batchbuffer.  Thus we should assert that that
doesn't happen, which means that the flush is only needed on first map
of the buffer."

I was going to comment that the first patch change behavior. Having the above text as a comment in the code is a good idea.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to