Re: [Mesa-dev] [PATCH] intel: Replace intel_renderbuffer::region with a miptree [v2]
On 11/18/2011 05:03 PM, Eric Anholt wrote:
On Fri, 18 Nov 2011 12:50:36 -0800, Chad Versace
chad.vers...@linux.intel.com wrote:
Essentially, this patch just globally substitutes `irb-region` with
`irb-mt-region` and then does some minor cleanups to avoid segfaults
and other problems.
This is in preparation for
1. Fixing scatter/gather for mipmapped separate stencil textures.
2. Supporting HiZ for mipmapped depth textures.
As a nice benefit, this lays down some preliminary groundwork for easily
texturing from any renderbuffer, even those of the window system.
A future commit will replace intel_mipmap_tree::hiz_region with a miptree.
v2:
- Return early in intel_process_dri2_buffer_*() if region allocation
fails.
- Fix double semicolon.
- Fix miptree reference leaks in the following functions:
intel_process_dri2_buffer_with_separate_stencil()
intel_image_target_renderbuffer_storage()
@@ -702,20 +696,21 @@ intel_alloc_renderbuffer_storage(struct gl_context *
ctx, struct gl_renderbuffer
_mesa_reference_renderbuffer(irb-wrapped_stencil, stencil_rb);
} else {
- irb-region = intel_region_alloc(intel-intelScreen, tiling, cpp,
- width, height, true);
- if (!irb-region)
+ irb-mt = intel_miptree_create_for_renderbuffer(intel, rb-Format,
+ tiling, cpp,
+ width, height);
+ if (!irb-mt)
return false;
if (intel-vtbl.is_hiz_depth_format(intel, rb-Format)) {
- irb-hiz_region = intel_region_alloc(intel-intelScreen,
- I915_TILING_Y,
- irb-region-cpp,
- irb-region-width,
- irb-region-height,
- true);
- if (!irb-hiz_region) {
-intel_region_release(irb-region);
+ irb-mt-hiz_region = intel_region_alloc(intel-intelScreen,
+ I915_TILING_Y,
+ cpp,
+ rb-Width,
+ rb-Height,
+ true);
+ if (!irb-mt) {
+intel_miptree_release(irb-mt);
return false;
I think this was meant to be if (!irb-mt-his_region).
Other than that,
Reviewed-by: Eric Anholt e...@anholt.net
I really meant to release irb-mt, not irb-mt-hiz_mt. Perhaps rephrasing
the code like this reveals the code's intention more explicity.
irb-mt = intel_miptree_create_for_renderbuffer(...)
if (!irb-mt)
return false;
if (intel-vtbl.is_hiz_depth_format(intel, rb-Format)) {
intel_miptree_alloc_hiz(intel, irb-mt);
if (!irb-mt-hiz_mt) {
intel_miptree_release(irb-mt);
return false;
}
}
I chose to release the renderbuffer when the hiz miptree allocation fails
because on gen6, hiz and separate stencil must be enabled together. If,
when hiz miptree allocation failed, we allowed intel_alloc_rb_storage()
to continue and succeed, this problematic situation could occur:
1. User creates a GL_DEPTH_COMPONENT24 renderbuffer. HiZ miptree allocation
failed.
2. User attaches the depthbuffer to a fb and does some drawing. No problem.
3. User creates a GL_STENCIL_INDEX8 renderbuffer.
4. User attaches the stencilbuffer to the same fb. Here we must to detect
that the accompanying depthbuffer, without hiz, is incompatible, and
mark the fb as incomplete.
5. User says WTF.
I'm withholding your rb until you reply.
Chad Versace
chad.vers...@linux.intel.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: Replace intel_renderbuffer::region with a miptree [v2]
On 11/21/2011 11:27 AM, Chad Versace wrote:
On 11/18/2011 05:03 PM, Eric Anholt wrote:
On Fri, 18 Nov 2011 12:50:36 -0800, Chad Versace
chad.vers...@linux.intel.com wrote:
Essentially, this patch just globally substitutes `irb-region` with
`irb-mt-region` and then does some minor cleanups to avoid segfaults
and other problems.
This is in preparation for
1. Fixing scatter/gather for mipmapped separate stencil textures.
2. Supporting HiZ for mipmapped depth textures.
As a nice benefit, this lays down some preliminary groundwork for easily
texturing from any renderbuffer, even those of the window system.
A future commit will replace intel_mipmap_tree::hiz_region with a miptree.
v2:
- Return early in intel_process_dri2_buffer_*() if region allocation
fails.
- Fix double semicolon.
- Fix miptree reference leaks in the following functions:
intel_process_dri2_buffer_with_separate_stencil()
intel_image_target_renderbuffer_storage()
@@ -702,20 +696,21 @@ intel_alloc_renderbuffer_storage(struct gl_context *
ctx, struct gl_renderbuffer
_mesa_reference_renderbuffer(irb-wrapped_stencil, stencil_rb);
} else {
- irb-region = intel_region_alloc(intel-intelScreen, tiling, cpp,
- width, height, true);
- if (!irb-region)
+ irb-mt = intel_miptree_create_for_renderbuffer(intel, rb-Format,
+ tiling, cpp,
+ width, height);
+ if (!irb-mt)
return false;
if (intel-vtbl.is_hiz_depth_format(intel, rb-Format)) {
-irb-hiz_region = intel_region_alloc(intel-intelScreen,
- I915_TILING_Y,
- irb-region-cpp,
- irb-region-width,
- irb-region-height,
- true);
-if (!irb-hiz_region) {
- intel_region_release(irb-region);
+irb-mt-hiz_region = intel_region_alloc(intel-intelScreen,
+ I915_TILING_Y,
+ cpp,
+ rb-Width,
+ rb-Height,
+ true);
+if (!irb-mt) {
+ intel_miptree_release(irb-mt);
return false;
I think this was meant to be if (!irb-mt-his_region).
Other than that,
Reviewed-by: Eric Anholt e...@anholt.net
I really meant to release irb-mt, not irb-mt-hiz_mt. Perhaps rephrasing
the code like this reveals the code's intention more explicity.
irb-mt = intel_miptree_create_for_renderbuffer(...)
if (!irb-mt)
return false;
if (intel-vtbl.is_hiz_depth_format(intel, rb-Format)) {
intel_miptree_alloc_hiz(intel, irb-mt);
if (!irb-mt-hiz_mt) {
intel_miptree_release(irb-mt);
return false;
}
}
I chose to release the renderbuffer when the hiz miptree allocation fails
because on gen6, hiz and separate stencil must be enabled together. If,
when hiz miptree allocation failed, we allowed intel_alloc_rb_storage()
to continue and succeed, this problematic situation could occur:
1. User creates a GL_DEPTH_COMPONENT24 renderbuffer. HiZ miptree
allocation failed.
2. User attaches the depthbuffer to a fb and does some drawing. No problem.
3. User creates a GL_STENCIL_INDEX8 renderbuffer.
4. User attaches the stencilbuffer to the same fb. Here we must to detect
that the accompanying depthbuffer, without hiz, is incompatible, and
mark the fb as incomplete.
5. User says WTF.
I'm withholding your rb until you reply.
Oops. I fixed that hunk to this
if (!irb-mt-hiz_region) {
intel_miptree_release(irb-mt);
return false;
}
and now accept your rb.
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: Replace intel_renderbuffer::region with a miptree [v2]
Essentially, this patch just globally substitutes `irb-region` with
`irb-mt-region` and then does some minor cleanups to avoid segfaults
and other problems.
This is in preparation for
1. Fixing scatter/gather for mipmapped separate stencil textures.
2. Supporting HiZ for mipmapped depth textures.
As a nice benefit, this lays down some preliminary groundwork for easily
texturing from any renderbuffer, even those of the window system.
A future commit will replace intel_mipmap_tree::hiz_region with a miptree.
v2:
- Return early in intel_process_dri2_buffer_*() if region allocation
fails.
- Fix double semicolon.
- Fix miptree reference leaks in the following functions:
intel_process_dri2_buffer_with_separate_stencil()
intel_image_target_renderbuffer_storage()
CC: Eric Anholt e...@anholt.net
Signed-off-by: Chad Versace chad.vers...@linux.intel.com
---
src/mesa/drivers/dri/i965/brw_misc_state.c| 21 ++-
src/mesa/drivers/dri/i965/brw_vtbl.c |2 +-
src/mesa/drivers/dri/i965/brw_wm_surface_state.c |2 +-
src/mesa/drivers/dri/i965/gen7_misc_state.c | 11 +-
src/mesa/drivers/dri/i965/gen7_wm_surface_state.c |2 +-
src/mesa/drivers/dri/intel/intel_blit.c | 21 ++-
src/mesa/drivers/dri/intel/intel_buffer_objects.c |8 +-
src/mesa/drivers/dri/intel/intel_buffers.c|9 +-
src/mesa/drivers/dri/intel/intel_context.c| 78 ---
src/mesa/drivers/dri/intel/intel_fbo.c| 151 +++--
src/mesa/drivers/dri/intel/intel_fbo.h|7 +-
src/mesa/drivers/dri/intel/intel_pixel_copy.c |5 +-
src/mesa/drivers/dri/intel/intel_screen.c |3 +-
src/mesa/drivers/dri/intel/intel_span.c |3 +-
src/mesa/drivers/dri/intel/intel_tex_copy.c | 14 ++-
src/mesa/drivers/dri/intel/intel_tex_image.c |6 +-
16 files changed, 203 insertions(+), 140 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 514c990..4119afa 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -33,6 +33,7 @@
#include intel_batchbuffer.h
#include intel_fbo.h
+#include intel_mipmap_tree.h
#include intel_regions.h
#include brw_context.h
@@ -204,9 +205,14 @@ static void emit_depthbuffer(struct brw_context *brw)
/* _NEW_BUFFERS */
struct intel_renderbuffer *depth_irb = intel_get_renderbuffer(fb,
BUFFER_DEPTH);
struct intel_renderbuffer *stencil_irb = intel_get_renderbuffer(fb,
BUFFER_STENCIL);
- struct intel_region *hiz_region = depth_irb ? depth_irb-hiz_region : NULL;
+ struct intel_region *hiz_region = NULL;
unsigned int len;
+ if (depth_irb
+ depth_irb-mt) {
+ hiz_region = depth_irb-mt-hiz_region;
+ }
+
/* 3DSTATE_DEPTH_BUFFER, 3DSTATE_STENCIL_BUFFER are both
* non-pipelined state that will need the PIPE_CONTROL workaround.
*/
@@ -272,6 +278,8 @@ static void emit_depthbuffer(struct brw_context *brw)
* [DevGT]: This field must be set to the same value (enabled or
* disabled) as Hierarchical Depth Buffer Enable
*/
+ struct intel_region *region = stencil_irb-mt-region;
+
assert(intel-has_separate_stencil);
assert(stencil_irb-Base.Format == MESA_FORMAT_S8);
@@ -283,8 +291,8 @@ static void emit_depthbuffer(struct brw_context *brw)
(BRW_TILEWALK_YMAJOR 26) |
(BRW_SURFACE_2D 29));
OUT_BATCH(0);
- OUT_BATCH(((stencil_irb-region-width - 1) 6) |
-(2 * stencil_irb-region-height - 1) 19);
+ OUT_BATCH(((region-width - 1) 6) |
+(2 * region-height - 1) 19);
OUT_BATCH(0);
OUT_BATCH(0);
@@ -294,7 +302,7 @@ static void emit_depthbuffer(struct brw_context *brw)
ADVANCE_BATCH();
} else {
- struct intel_region *region = depth_irb-region;
+ struct intel_region *region = depth_irb-mt-region;
unsigned int format;
uint32_t tile_x, tile_y, offset;
@@ -379,10 +387,11 @@ static void emit_depthbuffer(struct brw_context *brw)
/* Emit stencil buffer. */
if (stencil_irb) {
+struct intel_region *region = stencil_irb-mt-region;
BEGIN_BATCH(3);
OUT_BATCH((_3DSTATE_STENCIL_BUFFER 16) | (3 - 2));
-OUT_BATCH(stencil_irb-region-pitch * stencil_irb-region-cpp - 1);
-OUT_RELOC(stencil_irb-region-bo,
+OUT_BATCH(region-pitch * region-cpp - 1);
+OUT_RELOC(region-bo,
I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
0);
ADVANCE_BATCH();
diff --git a/src/mesa/drivers/dri/i965/brw_vtbl.c
b/src/mesa/drivers/dri/i965/brw_vtbl.c
index 7c23faa..9302b27 100644
--- a/src/mesa/drivers/dri/i965/brw_vtbl.c
+++ b/src/mesa/drivers/dri/i965/brw_vtbl.c
@@ -126,7 +126,7 @@ brw_update_draw_buffer(struct intel_context *intel)
/* Check
Re: [Mesa-dev] [PATCH] intel: Replace intel_renderbuffer::region with a miptree [v2]
On Fri, 18 Nov 2011 12:50:36 -0800, Chad Versace chad.vers...@linux.intel.com
wrote:
Essentially, this patch just globally substitutes `irb-region` with
`irb-mt-region` and then does some minor cleanups to avoid segfaults
and other problems.
This is in preparation for
1. Fixing scatter/gather for mipmapped separate stencil textures.
2. Supporting HiZ for mipmapped depth textures.
As a nice benefit, this lays down some preliminary groundwork for easily
texturing from any renderbuffer, even those of the window system.
A future commit will replace intel_mipmap_tree::hiz_region with a miptree.
v2:
- Return early in intel_process_dri2_buffer_*() if region allocation
fails.
- Fix double semicolon.
- Fix miptree reference leaks in the following functions:
intel_process_dri2_buffer_with_separate_stencil()
intel_image_target_renderbuffer_storage()
@@ -702,20 +696,21 @@ intel_alloc_renderbuffer_storage(struct gl_context *
ctx, struct gl_renderbuffer
_mesa_reference_renderbuffer(irb-wrapped_stencil, stencil_rb);
} else {
- irb-region = intel_region_alloc(intel-intelScreen, tiling, cpp,
-width, height, true);
- if (!irb-region)
+ irb-mt = intel_miptree_create_for_renderbuffer(intel, rb-Format,
+ tiling, cpp,
+ width, height);
+ if (!irb-mt)
return false;
if (intel-vtbl.is_hiz_depth_format(intel, rb-Format)) {
- irb-hiz_region = intel_region_alloc(intel-intelScreen,
- I915_TILING_Y,
- irb-region-cpp,
- irb-region-width,
- irb-region-height,
- true);
- if (!irb-hiz_region) {
- intel_region_release(irb-region);
+ irb-mt-hiz_region = intel_region_alloc(intel-intelScreen,
+ I915_TILING_Y,
+ cpp,
+ rb-Width,
+ rb-Height,
+ true);
+ if (!irb-mt) {
+ intel_miptree_release(irb-mt);
return false;
I think this was meant to be if (!irb-mt-his_region).
Other than that,
Reviewed-by: Eric Anholt e...@anholt.net
pgpjrqfOfvvBA.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
