Re: [Mesa-dev] [PATCH 17/41] intel: Replace intel_mipmap_tree::hiz_region with a miptree

2011-11-18 Thread Eric Anholt 
On Thu, 17 Nov 2011 19:58:44 -0800, Chad Versace chad.vers...@linux.intel.com 
wrote:
 This is required to correctly implement HiZ for mipmapped and
 multi-layered textures.
 
 Signed-off-by: Chad Versace chad.vers...@linux.intel.com
 ---
  src/mesa/drivers/dri/i965/brw_misc_state.c |5 +++--
  src/mesa/drivers/dri/intel/intel_context.c |   20 ++--
  src/mesa/drivers/dri/intel/intel_fbo.c |   11 +++
  src/mesa/drivers/dri/intel/intel_mipmap_tree.c |   19 ++-
  src/mesa/drivers/dri/intel/intel_mipmap_tree.h |   22 +-
  5 files changed, 47 insertions(+), 30 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c 
 b/src/mesa/drivers/dri/i965/brw_misc_state.c
 index 4119afa..17da460 100644
 --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
 +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c

 @@ -1394,17 +1394,17 @@ 
 intel_process_dri2_buffer_with_separate_stencil(struct intel_context *intel,
   buffer-pitch / buffer-cpp,
   buffer-name,
   buffer_name);
 -
 +   struct intel_mipmap_tree *mt =
 +  intel_miptree_create_for_region(intel,
 +   GL_TEXTURE_2D,
 +   rb-Base.Format,
 +   region);
 +   intel_region_release(region);
 if (buffer-attachment == __DRI_BUFFER_HIZ) {
 -  intel_region_reference(rb-mt-hiz_region, region);
 +  rb-mt-hiz_mt = mt;
 } else {
 -  rb-mt = intel_miptree_create_for_region(intel,
 -   GL_TEXTURE_2D,
 -   rb-Base.Format,
 -   region);;
 +  rb-mt = mt;
 }

I don't see who would have already released preexisting rb-mt here.

 +bool
 +intel_miptree_alloc_hiz(struct intel_context *intel,
 + struct intel_mipmap_tree *mt)
 +{
 +   assert(mt-hiz_mt == NULL);
 +   mt-hiz_mt = intel_miptree_create(intel,
 + mt-target,
 + MESA_FORMAT_X8_Z24,
 + mt-first_level,
 + mt-last_level,
 + mt-width0,
 + mt-height0,
 + mt-depth0,
 + true);
 +   return mt-hiz_mt != NULL;
 +}

We don't get any size reduction on the hiz miptree compared to the real
miptree?


pgpNPtGmGA6G8.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 17/41] intel: Replace intel_mipmap_tree::hiz_region with a miptree

2011-11-18 Thread Chad Versace 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 11/18/2011 03:24 PM, Eric Anholt wrote:
 On Thu, 17 Nov 2011 19:58:44 -0800, Chad Versace 
 chad.vers...@linux.intel.com wrote:
 This is required to correctly implement HiZ for mipmapped and
 multi-layered textures.

 Signed-off-by: Chad Versace chad.vers...@linux.intel.com
 ---
  src/mesa/drivers/dri/i965/brw_misc_state.c |5 +++--
  src/mesa/drivers/dri/intel/intel_context.c |   20 ++--
  src/mesa/drivers/dri/intel/intel_fbo.c |   11 +++
  src/mesa/drivers/dri/intel/intel_mipmap_tree.c |   19 ++-
  src/mesa/drivers/dri/intel/intel_mipmap_tree.h |   22 +-
  5 files changed, 47 insertions(+), 30 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c 
 b/src/mesa/drivers/dri/i965/brw_misc_state.c
 index 4119afa..17da460 100644
 --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
 +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
 
 @@ -1394,17 +1394,17 @@ 
 intel_process_dri2_buffer_with_separate_stencil(struct intel_context *intel,
  buffer-pitch / buffer-cpp,
  buffer-name,
  buffer_name);
 -
 +   struct intel_mipmap_tree *mt =
 +  intel_miptree_create_for_region(intel,
 +  GL_TEXTURE_2D,
 +  rb-Base.Format,
 +  region);
 +   intel_region_release(region);
 if (buffer-attachment == __DRI_BUFFER_HIZ) {
 -  intel_region_reference(rb-mt-hiz_region, region);
 +  rb-mt-hiz_mt = mt;
 } else {
 -  rb-mt = intel_miptree_create_for_region(intel,
 -   GL_TEXTURE_2D,
 -   rb-Base.Format,
 -   region);;
 +  rb-mt = mt;
 }
 
 I don't see who would have already released preexisting rb-mt here.

Fixed that in v2 of this patch, in response to your refcount comments
on the earlier patch.

 
 +bool
 +intel_miptree_alloc_hiz(struct intel_context *intel,
 +struct intel_mipmap_tree *mt)
 +{
 +   assert(mt-hiz_mt == NULL);
 +   mt-hiz_mt = intel_miptree_create(intel,
 + mt-target,
 + MESA_FORMAT_X8_Z24,
 + mt-first_level,
 + mt-last_level,
 + mt-width0,
 + mt-height0,
 + mt-depth0,
 + true);
 +   return mt-hiz_mt != NULL;
 +}
 
 We don't get any size reduction on the hiz miptree compared to the real
 miptree?

Yes and no.

No, because we always set LOD=0 and manually compute the draw offsets in
the SURFACE_STATE batch. The HiZ and stencil buffers inherit the draw
offset from 3DSTATE_DEPTH_BUFFER, so the HiZ and stencil miptree must have
identical layout to the depth miptree.

To save memory, we could waiver the identical-layout requirement and allocate
a small hiz and stencil miptrees for non-texture renderbuffers, since the draw
offsets will never be used in that case. But let's save that optimization for
a future patch.

Yes, we could allocate small hiz and stencil miptrees if (and that's a big if)
we used the LOD field in SURFACE_STATE. The hardware expects compact miptree
layout in that case.

An additional reason that we may want start using the LOD field is that, when
wrapping a renderbuffer around mipmapped or multi-layer depthstencil textures,
the stencil miptree layout will always be incorrect in the current way we do
things. We should discuss that problem f2f someday.

- 
Chad Versace
chad.vers...@linux.intel.com
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJOxu0OAAoJEAIvNt057x8ifzkQAL0OL3YqG2fy5qgdbov6410q
Lp2fiQhV3LehacvFeOuSTNWgjjtoSY8dAcqu/WvDQJcrWYiizofQ1dtHD0f1Q0r1
AggSfGW7d1Srd+sV2+HOWMsOtU+b0ePhVgzWmmjfWGO9inLIpnLEU7P0n4zcLK/z
ZP4TSCW/lyqkFrP4X/M4HWiqEpzfcIfwNrPchSzBsiG8Ut17hcMRGeqUS8I3ubFa
WMBDtJtRVBVLyb2j45L5MikjrY3TaCkOa+7Vbj2W6IQ2QstHoqPkQSBbIwDpUGF2
sGHnGsmO1eHbTe5XrSFo0xCpMjVhIZZ3oH09aK1UfEDcVcSQb6BFsM2G0xBj8bFY
c72yY9DhdfTrX+IuVDn9u7F9mt6fO5w2lxZtMPjw4XC7CziElbne123qQMDYUyVh
IQ9kW1t9qLMQlduCjfTFLZUijUGQtf6JdoFr7em5Z4B0l1dpwdGyRI/5mYYzOqaG
/gfwIpT/meMTz3Xl5On/nN0zGdQ9CLZLOuHMerEIzuJVv7SCbiLeB195hrGCLbBe
ZE3yNMOMHOfqT4o5C5hmk/UWnsLYTKFXRdD2ZFrp9jkQ/isVkBVtVs/xgMUvLknL
+T9sIUGpi32gHL+Mw0qorApQN56C/yFxU72NS3gK1KefbNmI8WKCL+lz4KFYqds6
TUuGTo5LCt28iAS4V5qe
=S1Yx
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 17/41] intel: Replace intel_mipmap_tree::hiz_region with a miptree

2011-11-18 Thread Eric Anholt 
On Fri, 18 Nov 2011 15:41:04 -0800, Chad Versace chad.vers...@linux.intel.com 
wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 On 11/18/2011 03:24 PM, Eric Anholt wrote:
  On Thu, 17 Nov 2011 19:58:44 -0800, Chad Versace 
  chad.vers...@linux.intel.com wrote:
  +bool
  +intel_miptree_alloc_hiz(struct intel_context *intel,
  +  struct intel_mipmap_tree *mt)
  +{
  +   assert(mt-hiz_mt == NULL);
  +   mt-hiz_mt = intel_miptree_create(intel,
  + mt-target,
  + MESA_FORMAT_X8_Z24,
  + mt-first_level,
  + mt-last_level,
  + mt-width0,
  + mt-height0,
  + mt-depth0,
  + true);
  +   return mt-hiz_mt != NULL;
  +}
  
  We don't get any size reduction on the hiz miptree compared to the real
  miptree?
 
 Yes and no.
 
 No, because we always set LOD=0 and manually compute the draw offsets in
 the SURFACE_STATE batch. The HiZ and stencil buffers inherit the draw
 offset from 3DSTATE_DEPTH_BUFFER, so the HiZ and stencil miptree must have
 identical layout to the depth miptree.
 
 To save memory, we could waiver the identical-layout requirement and allocate
 a small hiz and stencil miptrees for non-texture renderbuffers, since the draw
 offsets will never be used in that case. But let's save that optimization for
 a future patch.
 
 Yes, we could allocate small hiz and stencil miptrees if (and that's a big if)
 we used the LOD field in SURFACE_STATE. The hardware expects compact miptree
 layout in that case.

Good enough answer for me.


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

[Mesa-dev] [PATCH 17/41] intel: Replace intel_mipmap_tree::hiz_region with a miptree

2011-11-17 Thread Chad Versace 
This is required to correctly implement HiZ for mipmapped and
multi-layered textures.

Signed-off-by: Chad Versace chad.vers...@linux.intel.com
---
 src/mesa/drivers/dri/i965/brw_misc_state.c |5 +++--
 src/mesa/drivers/dri/intel/intel_context.c |   20 ++--
 src/mesa/drivers/dri/intel/intel_fbo.c |   11 +++
 src/mesa/drivers/dri/intel/intel_mipmap_tree.c |   19 ++-
 src/mesa/drivers/dri/intel/intel_mipmap_tree.h |   22 +-
 5 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c 
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 4119afa..17da460 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -209,8 +209,9 @@ static void emit_depthbuffer(struct brw_context *brw)
unsigned int len;
 
if (depth_irb 
-   depth_irb-mt) {
-  hiz_region = depth_irb-mt-hiz_region;
+   depth_irb-mt 
+   depth_irb-mt-hiz_mt) {
+  hiz_region = depth_irb-mt-hiz_mt-region;
}
 
/* 3DSTATE_DEPTH_BUFFER, 3DSTATE_STENCIL_BUFFER are both
diff --git a/src/mesa/drivers/dri/intel/intel_context.c 
b/src/mesa/drivers/dri/intel/intel_context.c
index 681bf8b..bc89bb1 100644
--- a/src/mesa/drivers/dri/intel/intel_context.c
+++ b/src/mesa/drivers/dri/intel/intel_context.c
@@ -1356,8 +1356,8 @@ intel_process_dri2_buffer_with_separate_stencil(struct 
intel_context *intel,
rb-mt-region-name == buffer-name) ||
(buffer-attachment == __DRI_BUFFER_HIZ 
rb-mt 
-   rb-mt-hiz_region 
-   rb-mt-hiz_region-name == buffer-name)) {
+   rb-mt-hiz_mt 
+   rb-mt-hiz_mt-region-name == buffer-name)) {
   return;
}
 
@@ -1394,17 +1394,17 @@ intel_process_dri2_buffer_with_separate_stencil(struct 
intel_context *intel,
buffer-pitch / buffer-cpp,
buffer-name,
buffer_name);
-
+   struct intel_mipmap_tree *mt =
+  intel_miptree_create_for_region(intel,
+ GL_TEXTURE_2D,
+ rb-Base.Format,
+ region);
+   intel_region_release(region);
if (buffer-attachment == __DRI_BUFFER_HIZ) {
-  intel_region_reference(rb-mt-hiz_region, region);
+  rb-mt-hiz_mt = mt;
} else {
-  rb-mt = intel_miptree_create_for_region(intel,
-   GL_TEXTURE_2D,
-   rb-Base.Format,
-   region);;
+  rb-mt = mt;
}
-
-   intel_region_release(region);
 }
 
 /**
diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c 
b/src/mesa/drivers/dri/intel/intel_fbo.c
index 7dc0c53..65b42ec 100644
--- a/src/mesa/drivers/dri/intel/intel_fbo.c
+++ b/src/mesa/drivers/dri/intel/intel_fbo.c
@@ -63,7 +63,7 @@ intel_framebuffer_has_hiz(struct gl_framebuffer *fb)
struct intel_renderbuffer *rb = NULL;
if (fb)
   rb = intel_get_renderbuffer(fb, BUFFER_DEPTH);
-   return rb  rb-mt  rb-mt-hiz_region;
+   return rb  rb-mt  rb-mt-hiz_mt;
 }
 
 struct intel_region*
@@ -705,13 +705,8 @@ intel_alloc_renderbuffer_storage(struct gl_context * ctx, 
struct gl_renderbuffer
 return false;
 
   if (intel-vtbl.is_hiz_depth_format(intel, rb-Format)) {
-irb-mt-hiz_region = intel_region_alloc(intel-intelScreen,
- I915_TILING_Y,
- cpp,
- rb-Width,
- rb-Height,
- true);
-if (!irb-mt) {
+bool ok = intel_miptree_alloc_hiz(intel, irb-mt);
+if (!ok) {
intel_miptree_release(irb-mt);
return false;
 }
diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c 
b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
index 71fba8d..378e09c 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
@@ -261,8 +261,8 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
   DBG(%s deleting %p\n, __FUNCTION__, *mt);
 
   intel_region_release(((*mt)-region));
-  intel_region_release(((*mt)-hiz_region));
   intel_miptree_release((*mt)-stencil_mt);
+  intel_miptree_release((*mt)-hiz_mt);
 
   for (i = 0; i  MAX_TEXTURE_LEVELS; i++) {
 free((*mt)-level[i].slice);
@@ -568,3 +568,20 @@ intel_miptree_s8z24_gather(struct intel_context *intel,
 {
intel_miptree_s8z24_scattergather(intel, mt, level, layer, false);
 }
+
+bool
+intel_miptree_alloc_hiz(struct intel_context *intel,
+   struct intel_mipmap_tree *mt)
+{
+   assert(mt-hiz_mt == NULL);
+   mt-hiz_mt = intel_miptree_create(intel,
+