[Mesa-dev] [PATCH 1/3] i965: Don't double the depth width for separate-stencil-only rendering.

2011-11-28 Thread Eric Anholt
This looks to have been confused with pitch setup, which does have to
be doubled.  The width is inherited by separate stencil, and it should
naturally look like the setup in the block below involving a valid
depthbuffer.
---
 src/mesa/drivers/dri/i965/brw_misc_state.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c 
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index cb1405c..f6a5ad6 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -298,7 +298,7 @@ static void emit_depthbuffer(struct brw_context *brw)
(BRW_SURFACE_2D  29));
   OUT_BATCH(0);
   OUT_BATCH(((region-width - 1)  6) |
-(2 * region-height - 1)  19);
+(region-height - 1)  19);
   OUT_BATCH(0);
   OUT_BATCH(0);
 
-- 
1.7.7.3

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


Re: [Mesa-dev] [PATCH 1/3] i965: Don't double the depth width for separate-stencil-only rendering.

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

The code that this patch fixes is indeed correct, but not for the reason you 
believe.

On 11/28/2011 10:48 AM, Eric Anholt wrote:
 This looks to have been confused with pitch setup, which does have to
 be doubled.

It was intentional.

 The width is inherited by separate stencil, and it should
 naturally look like the setup in the block below involving a valid
 depthbuffer.

I disagree. The setup in the block below is different because
depth_irb-region-height is the depth buffer's height, but
stencil_irb-region-height is not the stencil buffer's height.
That's because of the hack we resort to when allocating stencil
buffers.

Many months ago, the hack looked like this:
irb-region = intel_region_alloc(...,
  rb-Format,
  I915_TILING_Y
  cpp * 2,
  width,
  (height + 1) / 2);

So, in the line ``((2 * region-height - 1)  19)``, I was attempting
to adjust for the divide-by-2. (Yes, there is a potential off-by-one
error here, but that's moot now).

Currently, the hack looks like this, both throughout Mesa and in
the DDX:
irb-mt = intel_miptree_create_for_rendebuffer(
  intel,
  rb-Format,
  I915_TILING_NONE,
  cpp * 2,
  ALIGN(width, 64),
  ALIGN((height + 1) / 2, 64));

So, it is now impossible to determine the renderbuffer's height from
stencil_irb-region-height. Instead, we should directly appeal to
stencil_irb-Base.Height.


 ---
  src/mesa/drivers/dri/i965/brw_misc_state.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c 
 b/src/mesa/drivers/dri/i965/brw_misc_state.c
 index cb1405c..f6a5ad6 100644
 --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
 +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
 @@ -298,7 +298,7 @@ static void emit_depthbuffer(struct brw_context *brw)
   (BRW_SURFACE_2D  29));
OUT_BATCH(0);
OUT_BATCH(((region-width - 1)  6) |
 -  (2 * region-height - 1)  19);
 +  (region-height - 1)  19);
OUT_BATCH(0);
OUT_BATCH(0);

I think this is the diff we want:

 -  OUT_BATCH(((region-width - 1)  6) |
 -  (2 * region-height - 1)  19);
 +  OUT_BATCH(((stencil_irb-Base.Width - 1)  6) |
 +  (stencil_irb-Base.Height - 1)  19);

- 
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/

iQIcBAEBAgAGBQJO1B1dAAoJEAIvNt057x8iCucP/iltzSWigV6G7rO4Ngg55j+8
ZEWEG5uLAv77ULqGdScoDupX3SSARvDU+dsKGbg1jL8vBg3lxZcHJji8Jb46/5pe
YnbKg/BSA4OQgt/GNBtYmjF+nFAipOqWxiQdaN8ytAGhraykOgJMTVx06Pw6+xZU
V/PRlRbLef7ECZT9FvI+XThiVJnyPq+OGQhpO6rYxXzUlRnutsLH0c1dohvoqHFh
qD6577KK0hHjVFK+duXg6Y1N5QPWlFi5PfqUbf8UcPBYv/tAHtYyfCghaDuqtN1w
uNTdv0w/2dQkgWVpBrRtZQIwGDRxoH4YN7aQi1gW4plWI6FED1XHmP5DllLH/4lI
W7foFGZb3raztYDjotTV/BstIcIXm1B0ASLv8ljjOjA17t0uZarZQKapLTNYMwd8
KUCtp2Sgr7EC7Teuck3tsaQLZxJinpkIWhqotajSNjWtdww5HG7Kdkc/zQDJORF8
Ri04nTZ94RMHOfGq3/+/9J/S5zFlCMbcDrFd9c7vCJ6bGrThHEEwe18HTXYJ/fOK
KlgrgTgEcmPKN32ExSkbZQm++XDenQEtQTo4Z9Nj8lQC51vWbcsPphXq3BWW8orc
ju/iTFmSRNXWDoL0W7PlL7pmfX2TuLTjZV0V4Ei/BUSQXt5oRx2n7VCExXQZvdOC
H9sZeKNNiQxm9VvlZxPT
=31Dx
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] i965: Don't double the depth width for separate-stencil-only rendering.

2011-11-28 Thread Eric Anholt
On Mon, 28 Nov 2011 15:46:38 -0800, Chad Versace chad.vers...@linux.intel.com 
wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 The code that this patch fixes is indeed correct, but not for the reason you 
 believe.
 
 On 11/28/2011 10:48 AM, Eric Anholt wrote:
  This looks to have been confused with pitch setup, which does have to
  be doubled.
 
 It was intentional.
 
  The width is inherited by separate stencil, and it should
  naturally look like the setup in the block below involving a valid
  depthbuffer.
 
 I disagree. The setup in the block below is different because
 depth_irb-region-height is the depth buffer's height, but
 stencil_irb-region-height is not the stencil buffer's height.
 That's because of the hack we resort to when allocating stencil
 buffers.

Tricky.  I just stumbled over this half an hour ago in the mapping
cleanup I'm doing, too, and had concluded that rb-Height was the thing
I wanted to use there.  I like your proposed hunk.


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