Re: [Mesa-dev] [PATCH 6/6] intel: Detile stencil buffer only if necessary

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

On 11/15/2011 06:16 PM, Eric Anholt wrote:
 On Sun, 13 Nov 2011 22:32:15 -0800, Chad Versace 
 chad.vers...@linux.intel.com wrote:
 In intel_map_renderbuffer_s8(), detile and copy the stencil buffer into
 the temporary buffer only if the renderbuffer is mapped in read mode. If
 the caller never going to read the bits but just clobber them, then it's
 wasted effort to detile the stencil buffer.

 CC: Eric Anholt e...@anholt.net
 Signed-off-by: Chad Versace chad.vers...@linux.intel.com
 
 I don't think this is correct.
 
 Imagine something that isn't writing all of the pixels, but definitely
 isn't reading data before writing.  It might sensibly ask for
 GL_WRITE_BIT but not GL_READ_BIT, and then all the pixels it doesn't
 write get trashed when the whole rectangle is written back out.
 
 If the mode contained GL_MAP_INVALIDATE_RANGE_BIT (basically,
 interpreting the mode argument like GL_ARB_map_buffer_range, which was
 my intent if not Brian's), this would be reasonable.

You're correct. This patch is nakd.

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

iQIcBAEBAgAGBQJOxCQDAAoJEAIvNt057x8iMnQQALJW9MK18IHdQ08RX0lxLDkW
Tzy5U3zv+j8NxzdQl4aQyA2lXmpNZHzPB6L3AV0Yog+I3ey63A0ADNi94brbu4LS
EX/tu+Vb0VsSTC9/aSazDDLQJbLecRo2ZdYueOn/3eKJONmHeCZz8XZrn5wgSE1W
Uh4dSR7cORw1vBZBugB5vD55JlRrZtDM45d6s/VRzY2O2ujrBQvr/lNbTZ35HNd7
zIL+67y50RBWMYIiyyzyXHa8lWVwllo1K9CM0LBmebxcX5t6rCn5QyZcT5F0DQAn
CyKpKBVDt2mkaOqgnFUiXSTETTOKCla9tzBjNS6OlI65wAmYRZZz0he1rrdl1g6Y
kKkI61qWwCOyKMLdRxCWoXYY3QmenIEe4++idtlTauFgKtSZE2v4oAx2X1Z4FvY8
vbcpje3tbt7khMQnFVrVU/nHIyKw6Ko2r+WP32Cr2j044f1VAj+N1zIpCNdPTsd6
ZvJWdAUcPM+vN9fXacmWJAheE2sfzXCJ9o6ffCpRs7LYSv2ZuVlpyZ53a06D5dl/
ya9A9dMbjmFr0MGfELWHKgyRv+Cx/0XxrgvmQNxz6O4TUp6V+xiE6Qiyr5kj90wI
bah5zurL3mRVTH0Dp/pQFOxiIIzovCTXGHi35ymRL1/rLWyhgApa2jngVqqeawEi
9Ea+IzVuGlyke/v2kocL
=PH5l
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 6/6] intel: Detile stencil buffer only if necessary

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

On 11/15/2011 06:17 PM, Eric Anholt wrote:
 On Tue, 15 Nov 2011 08:06:34 -0800, Chad Versace
 chad.vers...@linux.intel.com wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 On 11/14/2011 04:03 PM, Eric Anholt wrote:
 On Mon, 14 Nov 2011 14:29:51 -0800, Chad Versace
 chad.vers...@linux.intel.com wrote:
 Without 3, there were lots of bugs. But now that you ask, I don't
 remember
 exactly why the bug happened. I'm digging into that now.
 (I wrote the patch at midnight, and I dont' remember much from that
 hour...) 

 Ah!  It all makes sense now.

 I didn't guess that, because my midnight patches never have such good
 commit messages :)

 Do rb's for patches 5 and 6 come with your revelation?

 The patches really do fix bugs, and I'm feeling kinda lazy about splitting
 them up now.
 
 I thought the conclusion yesterday was that the lacks-explanation third
 of patch 5 was likely not required.  What happened, or am I
 misremembering?

Part 3 of patch 5 is required. I did some digging and discovered why. Let's
carefully walk through the reason.

Suppose that irb-wrapped_depth is installed at fb-_DepthBuffer and that
irb-wrapped_stencil is installed at fb-_StencilBuffer; that is, this patch
is applied without part 3.

When swrast_render_start is called, in general it is impossible to know which
buffers its caller will access. So we must map all three of
fb-Attachment[BUFFER_DEPTH]
fb-Attachment[BUFFER_DEPTH]-wrapped_depth   (which is fb-_DepthBuffer)
fb-Attachment[BUFFER_DEPTH]-wrapped_stencil (which is fb-_StencilBuffer)

When swrast_render_finish() is called, in general it is impossible to know
which buffers were accessed. So we must unmap  all three of the above buffers.
The order in which they are unmapped is signficant, because a scatter/gather
operation occurs for
intel_unmap_renderbuffer(fb-Attachment[BUFFER_DEPTH])
intel_unmap_renderbuffer(fb-Attachment[BUFFER_DEPTH]-wrapped_stencil)

Now consider two cases.

Case 1
- --
Suppose that swrast_render_finish's caller wrote to fb-_StencilBuffer and did
not access fb-Attachment[BUFFER_DEPTH]. Then we must perform the unmaps in
this order:
1. fb-Attachment[BUFFER_DEPTH]
2. fb-Attachment[BUFFER_DEPTH]-wrapped_stencil
Why? Unmap 1 scatters the s8 bits of the s8z24 buffer into the stencil buffer
Unnmap 2 copies the temporary untiled stencil buffer into the tiled stencil
buffer. Since fb-_StencilBuffer now holds the real stencil buffer contents,
unmap 2 needs to win.

Case 2
- --
Suppose that swrast_render_finish's caller does not access fb-_StencilBuffer
and writes to the stencil bits of fb-Attachment[BUFFER_DEPTH]. Then we must
perform the unmaps in this order:
1. fb-Attachment[BUFFER_DEPTH]-wrapped_stencil
2. fb-Attachment[BUFFER_DEPTH]


Since case 1 and 2 require conflicting orderings, intel_renderbuffer_unmap
must be incorrect. The only way to escape the problem is by part 3 of my
patch.

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

iQIcBAEBAgAGBQJOxE03AAoJEAIvNt057x8iDGYP/Rcd3uZRc18Vq+3Q4NK5UmMp
mGcGW0XMFxNSKhSGk0yoBgDT+LuhOY8WgAYK/qUbwK5Y3cL+zG8/vGJKb67Qp29w
8qsqrz/ONbEufDr6er+srOIGhM91lg23yW1wRAJkcnpxcoujVm2N4ljWyWigbShf
YywwbFbxVIIJq5+wskp4H1ENg/pjyCJGrCPKRsGF2B0+2k+Hnr/Cwnqr3ASI0lYt
S4CtfXrEkx1PvH3s8ZxxmXS2UFe6fcbgChEnvUko6Q0xTKzurr3Zam+35HKRWK0k
DmmF4KeGCgrOWL0fsdOaHYvtr2eB7S/4ZRZVN53L+mpJN+oRVv/JVDk2OnkWsx1n
/pNsPrDxzUt4bxV+qsiHwNLDTdgRneWKCT6NccRtQrRGnboh67L/Ebgw3UvsyLWi
xkMU3rJ2LeDVQVampGtzSZZl7KEZ8BjhgGUViGAxa2iG93TMRjHl0KwKHztvNLaT
7lIgmBE/A3jRSq7U8DqGscT7wkKJG7OBCwJZju1ezAqujdESfEYwrOayTLbiM9oz
tG4F1gtNIjPwyefdyZbdnKlKJ1d8u90xaBvKcx/Qi6Q3jSDawS1NKYC0QLZnwNwH
23JYsDNCiHAz5IrQYr2yYV/FejadGNVCNhMt8ERCtgvXKw3EiaaafmRt6WhdLzWs
Gm6Ncq/l64B0ykLVYWPm
=5YD9
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 6/6] intel: Detile stencil buffer only if necessary

2011-11-15 Thread Eric Anholt
On Sun, 13 Nov 2011 22:32:15 -0800, Chad Versace chad.vers...@linux.intel.com 
wrote:
 In intel_map_renderbuffer_s8(), detile and copy the stencil buffer into
 the temporary buffer only if the renderbuffer is mapped in read mode. If
 the caller never going to read the bits but just clobber them, then it's
 wasted effort to detile the stencil buffer.
 
 CC: Eric Anholt e...@anholt.net
 Signed-off-by: Chad Versace chad.vers...@linux.intel.com

I don't think this is correct.

Imagine something that isn't writing all of the pixels, but definitely
isn't reading data before writing.  It might sensibly ask for
GL_WRITE_BIT but not GL_READ_BIT, and then all the pixels it doesn't
write get trashed when the whole rectangle is written back out.

If the mode contained GL_MAP_INVALIDATE_RANGE_BIT (basically,
interpreting the mode argument like GL_ARB_map_buffer_range, which was
my intent if not Brian's), this would be reasonable.


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