Re: [Mesa-dev] [PATCH] intel: Don't try intel_miptree_map_blit if a region is already mapped

2012-01-27 Thread Eric Anholt 
On Thu, 26 Jan 2012 18:29:30 -0800, Ian Romanick i...@freedesktop.org wrote:
 From: Ian Romanick ian.d.roman...@intel.com
 
 Eventually this path leads to _intel_batchbuffer_flush.  The first
 thing there is an assertion that nothing is mapped.
 
 Fixes the afore mentioned assertion failure in piglit's
 fbo-mipmap-copypix, and is related to bug #43328.
 
 NOTE: This is a candidate for the 8.0 branch.
 
 Signed-off-by: Ian Romanick ian.d.roman...@intel.com
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43328

I think the assert is invalid and should be dropped instead.

(basically: the assert is assuming that if *anything* is mapped, the
batch has been flushed, while the reasonable and more common thing to
do, is to flush if your particular buffer was already in use).



pgph76OSdh4oj.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel: Don't try intel_miptree_map_blit if a region is already mapped

2012-01-27 Thread Chad Versace 
On 01/27/2012 12:07 AM, Eric Anholt wrote:
 On Thu, 26 Jan 2012 18:29:30 -0800, Ian Romanick i...@freedesktop.org 
 wrote:
 From: Ian Romanick ian.d.roman...@intel.com

 Eventually this path leads to _intel_batchbuffer_flush.  The first
 thing there is an assertion that nothing is mapped.

 Fixes the afore mentioned assertion failure in piglit's
 fbo-mipmap-copypix, and is related to bug #43328.

 NOTE: This is a candidate for the 8.0 branch.

 Signed-off-by: Ian Romanick ian.d.roman...@intel.com
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43328
 
 I think the assert is invalid and should be dropped instead.
 
 (basically: the assert is assuming that if *anything* is mapped, the
 batch has been flushed, while the reasonable and more common thing to
 do, is to flush if your particular buffer was already in use).

I'm ok with removing the assertion as long its intent is satisfied:
/* No batch should be emitted that uses a mapped region, because that would
 * cause the map to be incoherent with GPU rendering done by the
 * batchbuffer.
Since intel_miptree_{map,unmap}_* are flushing at the right time to avoid that,
I think we are satisfying that condition, and it's safe to remove the assertion.


Chad Versace
chad.vers...@linux.intel.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] intel: Don't try intel_miptree_map_blit if a region is already mapped

2012-01-26 Thread Ian Romanick 
From: Ian Romanick ian.d.roman...@intel.com

Eventually this path leads to _intel_batchbuffer_flush.  The first
thing there is an assertion that nothing is mapped.

Fixes the afore mentioned assertion failure in piglit's
fbo-mipmap-copypix, and is related to bug #43328.

NOTE: This is a candidate for the 8.0 branch.

Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43328
---
 src/mesa/drivers/dri/intel/intel_mipmap_tree.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c 
b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
index eae79c1..ae354cb 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
@@ -1055,7 +1055,8 @@ intel_miptree_map(struct intel_context *intel,
} else if (intel-gen = 6 
  !(mode  GL_MAP_WRITE_BIT) 
  !mt-compressed 
- mt-region-tiling == I915_TILING_X) {
+ mt-region-tiling == I915_TILING_X
+  intel-num_mapped_regions == 0) {
   intel_miptree_map_blit(intel, mt, map, level, slice);
} else {
   intel_miptree_map_gtt(intel, mt, map, level, slice);
-- 
1.7.6.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel: Don't try intel_miptree_map_blit if a region is already mapped

2012-01-26 Thread Chad Versace 
On 01/26/2012 06:29 PM, Ian Romanick wrote:
 From: Ian Romanick ian.d.roman...@intel.com
 
 Eventually this path leads to _intel_batchbuffer_flush.  The first
 thing there is an assertion that nothing is mapped.
 
 Fixes the afore mentioned assertion failure in piglit's
 fbo-mipmap-copypix, and is related to bug #43328.
 
 NOTE: This is a candidate for the 8.0 branch.
 
 Signed-off-by: Ian Romanick ian.d.roman...@intel.com
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43328
 ---
  src/mesa/drivers/dri/intel/intel_mipmap_tree.c |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c 
 b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
 index eae79c1..ae354cb 100644
 --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
 +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
 @@ -1055,7 +1055,8 @@ intel_miptree_map(struct intel_context *intel,
 } else if (intel-gen = 6 
 !(mode  GL_MAP_WRITE_BIT) 
 !mt-compressed 
 -   mt-region-tiling == I915_TILING_X) {
 +   mt-region-tiling == I915_TILING_X
 +intel-num_mapped_regions == 0) {
intel_miptree_map_blit(intel, mt, map, level, slice);
 } else {
intel_miptree_map_gtt(intel, mt, map, level, slice);


I'm surprised that assertion did not happen more frequently.

Reviewed-by: Chad Versace chad.vers...@linux.intel.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev