Re: [Mesa-dev] [PATCH 2/3] intel: Eliminate unneeded hiz resolves

2012-08-21 Thread Eric Anholt 
Roland Scheidegger srol...@vmware.com writes:
 Am 21.08.2012 03:51, schrieb Kenneth Graunke:
 Yeah, sadly.  (And seriously, whoever invented 1366x768 resolution
 panels clearly didn't talk the guys that required 8x4 alignment...)
 Can't you just force that alignment?
 (Not that I know if it would be worth it.)

Yeah, I'd been thinking about it, but the problem is how incredibly
unclear the 8x4 requirement is.  Is it 8x4 alignment of the dispatched
primitive, or 8x4 pixels actually lit and sent off to the CC?  Once
again, we rage at hardware docs describing a derived requirement without
describing the cause of that requirement.

If I can just leave my surface state the same size and just align my
prim's coordinates, that would be awesome, but I expect that's not the
case at all.  If I have to be able to actually write to all the pixels
8x4 blocks, then I have to go adjust surface states as well.



pgpd80JIn3Nd2.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 2/3] intel: Eliminate unneeded hiz resolves

2012-08-20 Thread Chad Versace 
On 08/17/2012 02:07 PM, Paul Berry wrote:
 On 14 August 2012 16:58, Chad Versace chad.vers...@linux.intel.com
 mailto:chad.vers...@linux.intel.com wrote:
 
 On creating a hiz miptree, we conservatively marked that each miptree
 slice needed a hiz resolve. But the resolves are unneeded when creating
 a non-texture miptree, so this patch removes them.
 
 This eliminates one hiz resolve per each creation of a non-texture depth
 miptree.  Hence, this eliminates many resolves when resizing a window.
 
 
 So, with this change, are the contents of the HiZ buffer uninitialized for a
 newly created (or resized) non-texture miptree?  The HW docs don't specify the
 exact format of the HiZ buffer, so it's possible that there may be some 
 possible
 states of the HiZ buffer that are invalid.  If there are, and the hardware
 doesn't deal with those invalid states well, then we run the risk of sporadic
 incorrect results (if subsequent rendering fails to bring the HiZ buffer into 
 a
 valid state) or possibly sporadic GPU hangs (if the hardware is unable to cope
 with the invalid states).
 
 Alternatively, perhaps we could initialize non-texture HiZ buffers to a state
 that indicates that they need a fast depth clear rather than a HiZ resolve. 
 Clears are likely to be much faster than HiZ resolves, because they don't
 require reading from the depth buffer.  So we would still get most of the
 benefit of avoiding the HiZ resolve, and no risk of undefined behaviour.  In
 fact, we might conceivably get even more benefit, since after a fast depth
 clear, the HiZ buffer is in a state where future rendering is very efficient
 (since the depth buffer doesn't need to be read).

It's not always possible to do a fast depth clear due to size restrictions (the
renderbuffer dimensions must be aligned to 8x4).


You make a strong point. I don't wish to risk GPU hangs or incorrect rendering
for the sake of improving the performance of resizing windows. So I'll just drop
this patch.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/3] intel: Eliminate unneeded hiz resolves

2012-08-20 Thread Kenneth Graunke 

On 08/20/2012 04:10 PM, Chad Versace wrote:

On 08/17/2012 02:07 PM, Paul Berry wrote:

On 14 August 2012 16:58, Chad Versace chad.vers...@linux.intel.com
mailto:chad.vers...@linux.intel.com wrote:

 On creating a hiz miptree, we conservatively marked that each miptree
 slice needed a hiz resolve. But the resolves are unneeded when creating
 a non-texture miptree, so this patch removes them.

 This eliminates one hiz resolve per each creation of a non-texture depth
 miptree.  Hence, this eliminates many resolves when resizing a window.


So, with this change, are the contents of the HiZ buffer uninitialized for a
newly created (or resized) non-texture miptree?  The HW docs don't specify the
exact format of the HiZ buffer, so it's possible that there may be some possible
states of the HiZ buffer that are invalid.  If there are, and the hardware
doesn't deal with those invalid states well, then we run the risk of sporadic
incorrect results (if subsequent rendering fails to bring the HiZ buffer into a
valid state) or possibly sporadic GPU hangs (if the hardware is unable to cope
with the invalid states).

Alternatively, perhaps we could initialize non-texture HiZ buffers to a state
that indicates that they need a fast depth clear rather than a HiZ resolve.
Clears are likely to be much faster than HiZ resolves, because they don't
require reading from the depth buffer.  So we would still get most of the
benefit of avoiding the HiZ resolve, and no risk of undefined behaviour.  In
fact, we might conceivably get even more benefit, since after a fast depth
clear, the HiZ buffer is in a state where future rendering is very efficient
(since the depth buffer doesn't need to be read).


It's not always possible to do a fast depth clear due to size restrictions (the
renderbuffer dimensions must be aligned to 8x4).


Yeah, sadly.  (And seriously, whoever invented 1366x768 resolution 
panels clearly didn't talk the guys that required 8x4 alignment...)



You make a strong point. I don't wish to risk GPU hangs or incorrect rendering
for the sake of improving the performance of resizing windows. So I'll just drop
this patch.


To play devil's advocate here, no one has presented any concrete 
evidence that the GPU actually does have a problem with this.  What if 
it did turn out to just be paranoia?  I'd hate to leave a bunch of 
overhead and needlessly lower performance or power.

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

Re: [Mesa-dev] [PATCH 2/3] intel: Eliminate unneeded hiz resolves

2012-08-17 Thread Paul Berry 
On 14 August 2012 16:58, Chad Versace chad.vers...@linux.intel.com wrote:

 On creating a hiz miptree, we conservatively marked that each miptree
 slice needed a hiz resolve. But the resolves are unneeded when creating
 a non-texture miptree, so this patch removes them.

 This eliminates one hiz resolve per each creation of a non-texture depth
 miptree.  Hence, this eliminates many resolves when resizing a window.


So, with this change, are the contents of the HiZ buffer uninitialized for
a newly created (or resized) non-texture miptree?  The HW docs don't
specify the exact format of the HiZ buffer, so it's possible that there may
be some possible states of the HiZ buffer that are invalid.  If there are,
and the hardware doesn't deal with those invalid states well, then we run
the risk of sporadic incorrect results (if subsequent rendering fails to
bring the HiZ buffer into a valid state) or possibly sporadic GPU hangs (if
the hardware is unable to cope with the invalid states).

Alternatively, perhaps we could initialize non-texture HiZ buffers to a
state that indicates that they need a fast depth clear rather than a HiZ
resolve.  Clears are likely to be much faster than HiZ resolves, because
they don't require reading from the depth buffer.  So we would still get
most of the benefit of avoiding the HiZ resolve, and no risk of undefined
behaviour.  In fact, we might conceivably get even more benefit, since
after a fast depth clear, the HiZ buffer is in a state where future
rendering is very efficient (since the depth buffer doesn't need to be
read).



 Signed-off-by: Chad Versace chad.vers...@linux.intel.com
 ---
  src/mesa/drivers/dri/intel/intel_fbo.c |  4 ++-
  src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 39
 +-
  src/mesa/drivers/dri/intel/intel_mipmap_tree.h |  3 +-
  3 files changed, 30 insertions(+), 16 deletions(-)

 diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c
 b/src/mesa/drivers/dri/intel/intel_fbo.c
 index 3a610c2..ada0f69 100644
 --- a/src/mesa/drivers/dri/intel/intel_fbo.c
 +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
 @@ -538,7 +538,9 @@ intel_renderbuffer_update_wrapper(struct intel_context
 *intel,

 if (mt-hiz_mt == NULL 
 intel-vtbl.is_hiz_depth_format(intel, rb-Format)) {
 -  intel_miptree_alloc_hiz(intel, mt, 0 /* num_samples */);
 +  intel_miptree_alloc_hiz(intel, mt,
 +  0 /*num_samples*/,
 +  true /*for_texture*/);
if (!mt-hiz_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 24cd9e9..143f2e3 100644
 --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
 +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
 @@ -467,7 +467,8 @@ intel_miptree_create_for_renderbuffer(struct
 intel_context *intel,
goto fail;

 if (intel-vtbl.is_hiz_depth_format(intel, format)) {
 -  ok = intel_miptree_alloc_hiz(intel, mt, num_samples);
 +  ok = intel_miptree_alloc_hiz(intel, mt, num_samples,
 +   false /*for_texture*/);
if (!ok)
   goto fail;
 }
 @@ -825,7 +826,8 @@ intel_miptree_alloc_mcs(struct intel_context *intel,
  bool
  intel_miptree_alloc_hiz(struct intel_context *intel,
 struct intel_mipmap_tree *mt,
 -GLuint num_samples)
 +GLuint num_samples,
 +bool for_texture)
  {
 assert(mt-hiz_mt == NULL);
 /* MSAA HiZ surfaces always use IMS layout. */
 @@ -844,18 +846,27 @@ intel_miptree_alloc_hiz(struct intel_context *intel,
 if (!mt-hiz_mt)
return false;

 -   /* Mark that all slices need a HiZ resolve. */
 -   struct intel_resolve_map *head = mt-hiz_map;
 -   for (int level = mt-first_level; level = mt-last_level; ++level) {
 -  for (int layer = 0; layer  mt-level[level].depth; ++layer) {
 -head-next = malloc(sizeof(*head-next));
 -head-next-prev = head;
 -head-next-next = NULL;
 -head = head-next;
 -
 -head-level = level;
 -head-layer = layer;
 -head-need = GEN6_HIZ_OP_HIZ_RESOLVE;
 +   if (for_texture) {
 +  /* Mark that all slices need a HiZ resolve. This is necessary for
 +   * renderbuffers that wrap textures because the user may have
 previously
 +   * uploaded texture data into the parent depth miptree.
 +   *
 +   * This is skipped for non-texture miptrees. In the non-texture
 case,
 +   * the depth miptree and the hiz miptree are created together, and
 hence
 +   * the content of each is undefined here.
 +   */
 +  struct intel_resolve_map *head = mt-hiz_map;
 +  for (int level = mt-first_level; level = mt-last_level; ++level)
 {
 + for (int layer = 0; layer  mt-level[level].depth; ++layer) {
 +head-next = malloc(sizeof(*head-next));
 +head-next-prev

[Mesa-dev] [PATCH 2/3] intel: Eliminate unneeded hiz resolves

2012-08-14 Thread Chad Versace 
On creating a hiz miptree, we conservatively marked that each miptree
slice needed a hiz resolve. But the resolves are unneeded when creating
a non-texture miptree, so this patch removes them.

This eliminates one hiz resolve per each creation of a non-texture depth
miptree.  Hence, this eliminates many resolves when resizing a window.

Signed-off-by: Chad Versace chad.vers...@linux.intel.com
---
 src/mesa/drivers/dri/intel/intel_fbo.c |  4 ++-
 src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 39 +-
 src/mesa/drivers/dri/intel/intel_mipmap_tree.h |  3 +-
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c 
b/src/mesa/drivers/dri/intel/intel_fbo.c
index 3a610c2..ada0f69 100644
--- a/src/mesa/drivers/dri/intel/intel_fbo.c
+++ b/src/mesa/drivers/dri/intel/intel_fbo.c
@@ -538,7 +538,9 @@ intel_renderbuffer_update_wrapper(struct intel_context 
*intel,
 
if (mt-hiz_mt == NULL 
intel-vtbl.is_hiz_depth_format(intel, rb-Format)) {
-  intel_miptree_alloc_hiz(intel, mt, 0 /* num_samples */);
+  intel_miptree_alloc_hiz(intel, mt,
+  0 /*num_samples*/,
+  true /*for_texture*/);
   if (!mt-hiz_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 24cd9e9..143f2e3 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
@@ -467,7 +467,8 @@ intel_miptree_create_for_renderbuffer(struct intel_context 
*intel,
   goto fail;
 
if (intel-vtbl.is_hiz_depth_format(intel, format)) {
-  ok = intel_miptree_alloc_hiz(intel, mt, num_samples);
+  ok = intel_miptree_alloc_hiz(intel, mt, num_samples,
+   false /*for_texture*/);
   if (!ok)
  goto fail;
}
@@ -825,7 +826,8 @@ intel_miptree_alloc_mcs(struct intel_context *intel,
 bool
 intel_miptree_alloc_hiz(struct intel_context *intel,
struct intel_mipmap_tree *mt,
-GLuint num_samples)
+GLuint num_samples,
+bool for_texture)
 {
assert(mt-hiz_mt == NULL);
/* MSAA HiZ surfaces always use IMS layout. */
@@ -844,18 +846,27 @@ intel_miptree_alloc_hiz(struct intel_context *intel,
if (!mt-hiz_mt)
   return false;
 
-   /* Mark that all slices need a HiZ resolve. */
-   struct intel_resolve_map *head = mt-hiz_map;
-   for (int level = mt-first_level; level = mt-last_level; ++level) {
-  for (int layer = 0; layer  mt-level[level].depth; ++layer) {
-head-next = malloc(sizeof(*head-next));
-head-next-prev = head;
-head-next-next = NULL;
-head = head-next;
-
-head-level = level;
-head-layer = layer;
-head-need = GEN6_HIZ_OP_HIZ_RESOLVE;
+   if (for_texture) {
+  /* Mark that all slices need a HiZ resolve. This is necessary for
+   * renderbuffers that wrap textures because the user may have previously
+   * uploaded texture data into the parent depth miptree.
+   *
+   * This is skipped for non-texture miptrees. In the non-texture case,
+   * the depth miptree and the hiz miptree are created together, and hence
+   * the content of each is undefined here.
+   */
+  struct intel_resolve_map *head = mt-hiz_map;
+  for (int level = mt-first_level; level = mt-last_level; ++level) {
+ for (int layer = 0; layer  mt-level[level].depth; ++layer) {
+head-next = malloc(sizeof(*head-next));
+head-next-prev = head;
+head-next-next = NULL;
+head = head-next;
+
+head-level = level;
+head-layer = layer;
+head-need = GEN6_HIZ_OP_HIZ_RESOLVE;
+ }
   }
}
 
diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h 
b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
index 0d0e757..65743fd 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
@@ -503,7 +503,8 @@ intel_miptree_alloc_mcs(struct intel_context *intel,
 bool
 intel_miptree_alloc_hiz(struct intel_context *intel,
struct intel_mipmap_tree *mt,
-GLuint num_samples);
+GLuint num_samples,
+bool for_texture);
 
 void
 intel_miptree_slice_set_needs_hiz_resolve(struct intel_mipmap_tree *mt,
-- 
1.7.11.4

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