<div dir="ltr">On 28 May 2013 10:55, Eric Anholt <span dir="ltr"><<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">For a blit-uploaded temporary, it's faster on current hardware to memcpy<br>
the data into a linear CPU mapping than to go through the GTT.<br>
<br>
</div>v2: Turn the not-fully-supported mask into 3 supported enum values.<br>
<br>
Reviewed-and-tested-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> (v1)<br>
Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> (v1)<br>
---<br>
<br>
Paul wrote this in response to v1:<br>
<div class="im"><br>
> I'm not comfortable with the meaning of force_tiling_mask. Effectively at<br>
> the moment it's:<br>
><br>
> - 0 means all tiling formats are allowed<br>
> - (1 << n) means tiling format is required to be n<br>
> - any other value is undefined.<br>
><br>
> I'd prefer to see an "allowed_tiling_mask" where we set bit n if and only<br>
> if tiling mode n is allowed.<br>
<br>
</div>I was a bit uncomfortable too, but I didn't want to write unused,<br>
untested code to handle things like someone passing in<br>
I915_TILING_NONE | I915_TILING_Y. So here's a new variant that I<br>
think should avoid both our concerns.<br>
<br>
This was the only major post-review change to the code, the next<br>
biggest being the error handling in the next patch. Updated patches<br>
may be found at mtblit of my tree.<br></blockquote><div><br></div><div>I like this much better, thanks. This patch is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br>
<br>I haven't had a chance to review the rest of the series thoroughly, but I've satisfied myself that my fast color clear work will be able to rebase on top of it, so consider the series:<br><br>Acked-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
src/mesa/drivers/dri/intel/intel_fbo.c | 2 +-<br>
src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 25 +++++++++++++++++--------<br>
src/mesa/drivers/dri/intel/intel_mipmap_tree.h | 8 ++++++--<br>
<div class="im"> src/mesa/drivers/dri/intel/intel_tex_image.c | 2 +-<br>
src/mesa/drivers/dri/intel/intel_tex_validate.c | 2 +-<br>
</div> 5 files changed, 26 insertions(+), 13 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c b/src/mesa/drivers/dri/intel/intel_fbo.c<br>
index 05ff784..f75b635 100644<br>
<div class="im">--- a/src/mesa/drivers/dri/intel/intel_fbo.c<br>
+++ b/src/mesa/drivers/dri/intel/intel_fbo.c<br>
@@ -924,7 +924,7 @@ intel_renderbuffer_move_to_temp(struct intel_context *intel,<br>
width, height, depth,<br>
true,<br>
irb->mt->num_samples,<br>
- false /* force_y_tiling */);<br>
</div>+ INTEL_MIPTREE_TILING_ANY);<br>
<div class="im"><br>
if (intel->vtbl.is_hiz_depth_format(intel, new_mt->format)) {<br>
intel_miptree_alloc_hiz(intel, new_mt);<br>
diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c<br>
</div>index c3e55f4..9998300 100644<br>
<div class="im">--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c<br>
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c<br>
@@ -265,7 +265,7 @@ intel_miptree_create_layout(struct intel_context *intel,<br>
mt->logical_depth0,<br>
true,<br>
num_samples,<br>
- false /* force_y_tiling */);<br>
</div>+ INTEL_MIPTREE_TILING_ANY);<br>
<div class="im"> if (!mt->stencil_mt) {<br>
intel_miptree_release(&mt);<br>
return NULL;<br>
@@ -309,7 +309,7 @@ intel_miptree_choose_tiling(struct intel_context *intel,<br>
gl_format format,<br>
uint32_t width0,<br>
uint32_t num_samples,<br>
- bool force_y_tiling,<br>
</div>+ enum intel_miptree_tiling_mode requested,<br>
struct intel_mipmap_tree *mt)<br>
{<br>
<br>
@@ -320,8 +320,17 @@ intel_miptree_choose_tiling(struct intel_context *intel,<br>
<div class="im"> return I915_TILING_NONE;<br>
}<br>
<br>
- if (force_y_tiling)<br>
</div><div class="im">+ /* Some usages may want only one type of tiling, like depth miptrees (Y<br>
+ * tiled), or temporary BOs for uploading data once (linear).<br>
</div>+ */<br>
+ switch (requested) {<br>
+ case INTEL_MIPTREE_TILING_ANY:<br>
+ break;<br>
+ case INTEL_MIPTREE_TILING_Y:<br>
return I915_TILING_Y;<br>
+ case INTEL_MIPTREE_TILING_NONE:<br>
+ return I915_TILING_NONE;<br>
+ }<br>
<div class="im"><br>
if (num_samples > 1) {<br>
/* From p82 of the Sandy Bridge PRM, dw3[1] of SURFACE_STATE ("Tiled<br>
</div>@@ -375,7 +384,7 @@ intel_miptree_create(struct intel_context *intel,<br>
<div class="im"> GLuint depth0,<br>
bool expect_accelerated_upload,<br>
GLuint num_samples,<br>
- bool force_y_tiling)<br>
</div>+ enum intel_miptree_tiling_mode requested_tiling)<br>
<div class="im"> {<br>
struct intel_mipmap_tree *mt;<br>
gl_format tex_format = format;<br>
</div>@@ -441,7 +450,7 @@ intel_miptree_create(struct intel_context *intel,<br>
<div class="im"> }<br>
<br>
uint32_t tiling = intel_miptree_choose_tiling(intel, format, width0,<br>
- num_samples, force_y_tiling,<br>
</div>+ num_samples, requested_tiling,<br>
<div class="im"> mt);<br>
bool y_or_x = tiling == (I915_TILING_Y | I915_TILING_X);<br>
<br>
</div>@@ -570,7 +579,7 @@ intel_miptree_create_for_renderbuffer(struct intel_context *intel,<br>
<div class="im"><br>
mt = intel_miptree_create(intel, GL_TEXTURE_2D, format, 0, 0,<br>
width, height, depth, true, num_samples,<br>
- false /* force_y_tiling */);<br>
</div>+ INTEL_MIPTREE_TILING_ANY);<br>
if (!mt)<br>
goto fail;<br>
<br>
@@ -1008,7 +1017,7 @@ intel_miptree_alloc_mcs(struct intel_context *intel,<br>
<div class="im"> mt->logical_depth0,<br>
true,<br>
0 /* num_samples */,<br>
- true /* force_y_tiling */);<br>
</div>+ INTEL_MIPTREE_TILING_Y);<br>
<div class="im"><br>
/* From the Ivy Bridge PRM, Vol 2 Part 1 p326:<br>
*<br>
</div>@@ -1089,7 +1098,7 @@ intel_miptree_alloc_hiz(struct intel_context *intel,<br>
<div class="im"> mt->logical_depth0,<br>
true,<br>
mt->num_samples,<br>
- false /* force_y_tiling */);<br>
</div>+ INTEL_MIPTREE_TILING_ANY);<br>
<div class="im"><br>
if (!mt->hiz_mt)<br>
return false;<br>
diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h<br>
</div>index 543182a..4252128 100644<br>
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h<br>
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h<br>
@@ -387,7 +387,11 @@ struct intel_mipmap_tree<br>
GLuint refcount;<br>
};<br>
<br>
-<br>
+enum intel_miptree_tiling_mode {<br>
+ INTEL_MIPTREE_TILING_ANY,<br>
+ INTEL_MIPTREE_TILING_Y,<br>
+ INTEL_MIPTREE_TILING_NONE,<br>
+};<br>
<div class="im"><br>
struct intel_mipmap_tree *intel_miptree_create(struct intel_context *intel,<br>
</div> GLenum target,<br>
@@ -399,7 +403,7 @@ struct intel_mipmap_tree *intel_miptree_create(struct intel_context *intel,<br>
<div class="im"> GLuint depth0,<br>
bool expect_accelerated_upload,<br>
GLuint num_samples,<br>
- bool force_y_tiling);<br>
</div>+ enum intel_miptree_tiling_mode);<br>
<div class="im"><br>
struct intel_mipmap_tree *<br>
intel_miptree_create_layout(struct intel_context *intel,<br>
diff --git a/src/mesa/drivers/dri/intel/intel_tex_image.c b/src/mesa/drivers/dri/intel/intel_tex_image.c<br>
</div>index 4e307f8..d3e905b 100644<br>
<div class="im">--- a/src/mesa/drivers/dri/intel/intel_tex_image.c<br>
+++ b/src/mesa/drivers/dri/intel/intel_tex_image.c<br>
@@ -103,7 +103,7 @@ intel_miptree_create_for_teximage(struct intel_context *intel,<br>
depth,<br>
expect_accelerated_upload,<br>
intelImage->base.Base.NumSamples,<br>
- false /* force_y_tiling */);<br>
</div>+ INTEL_MIPTREE_TILING_ANY);<br>
<div class="im"> }<br>
<br>
/* XXX: Do this for TexSubImage also:<br>
diff --git a/src/mesa/drivers/dri/intel/intel_tex_validate.c b/src/mesa/drivers/dri/intel/intel_tex_validate.c<br>
</div>index eaa2561..a8a8647 100644<br>
<div class="im">--- a/src/mesa/drivers/dri/intel/intel_tex_validate.c<br>
+++ b/src/mesa/drivers/dri/intel/intel_tex_validate.c<br>
@@ -106,7 +106,7 @@ intel_finalize_mipmap_tree(struct intel_context *intel, GLuint unit)<br>
depth,<br>
true,<br>
0 /* num_samples */,<br>
- false /* force_y_tiling */);<br>
</div>+ INTEL_MIPTREE_TILING_ANY);<br>
<div class="HOEnZb"><div class="h5"> if (!intelObj->mt)<br>
return false;<br>
}<br>
--<br>
1.8.3.rc0<br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>