<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 22, 2016 at 7:03 PM, Jordan Justen <span dir="ltr"><<a href="mailto:jordan.l.justen@intel.com" target="_blank">jordan.l.justen@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">We rename do_blorp_blit() to try_blorp_blit(), and add a return error<br>
if the surface size for the blit is too large. Now, do_blorp_blit() is<br>
rewritten to try to split the blit into smaller operations if<br>
try_blorp_blit() fails.<br>
<br>
Note: In this commit, try_blorp_blit() will always attempt to blit and<br>
never return an error, which matches the previous behavior. We will<br>
enable the size checking and splitting in a future commit.<br>
<br>
The motivation for this splitting is that in some cases when we<br>
flatten an image, it's dimensions grow, and this can then exceed the<br>
programmable hardware limits. An example is w-tiled+MSAA blits.<br>
<br>
Signed-off-by: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>><br>
---<br>
src/intel/blorp/blorp_blit.c | 93 ++++++++++++++++++++++++++++++<wbr>+++++++++++---<br>
1 file changed, 87 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c<br>
index 24c4fd6..caa3e33 100644<br>
--- a/src/intel/blorp/blorp_blit.c<br>
+++ b/src/intel/blorp/blorp_blit.c<br>
@@ -1530,11 +1530,18 @@ struct blt_coords {<br>
struct blt_axis x, y;<br>
};<br>
<br>
-static void<br>
-do_blorp_blit(struct blorp_batch *batch,<br>
- struct blorp_params *params,<br>
- struct brw_blorp_blit_prog_key *wm_prog_key,<br>
- const struct blt_coords *coords)<br>
+#define BLIT_WIDTH_TOO_LARGE 1<br>
+#define BLIT_HEIGHT_TOO_LARGE 2<br></blockquote><div><br></div><div>Mind making this an enum? That way GDB will print out BLIT_WIDTH_TOO_LARGE | BLIT_HEIGHT_TOO_LARGE when your debugging.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+/* Try to blit. If the surface parameters exceed the size allowed by hardware,<br>
+ * then a bitmask of BLIT_WIDTH_TOO_LARGE and BLIT_HEIGHT_TOO_LARGE will be<br>
+ * returned. If 0 is returned, then the blit was successful.<br>
+ */<br>
+static unsigned<br>
+try_blorp_blit(struct blorp_batch *batch,<br>
+ struct blorp_params *params,<br>
+ struct brw_blorp_blit_prog_key *wm_prog_key,<br>
+ const struct blt_coords *coords)<br>
{<br>
const struct gen_device_info *devinfo = batch->blorp->isl_dev->info;<br>
<br>
@@ -1737,7 +1744,81 @@ do_blorp_blit(struct blorp_batch *batch,<br>
<br>
brw_blorp_get_blit_kernel(<wbr>batch->blorp, params, wm_prog_key);<br>
<br>
- batch->blorp->exec(batch, params);<br>
+ unsigned result = 0;<br>
+<br>
+ if (result == 0) {<br>
+ batch->blorp->exec(batch, params);<br>
+ }<br>
+<br>
+ return result;<br>
+}<br>
+<br>
+/* Adjust split blit source coordinates for the current destination<br>
+ * coordinates.<br>
+ */<br>
+static void<br>
+adjust_split_coords(const struct blt_axis *orig,<br>
+ struct blt_axis *split_coords,<br>
+ float scale)<br>
+{<br>
+ float delta0 = scale * (split_coords->dst0 - orig->dst0);<br>
+ float delta1 = scale * (split_coords->dst1 - orig->dst1);<br>
+ split_coords->src0 = orig->src0 + (scale >= 0.0 ? delta0 : delta1);<br>
+ split_coords->src1 = orig->src1 + (scale >= 0.0 ? delta1 : delta0);<br></blockquote><div><br></div><div>Ouch... My head hurts... I believe this is probably correct. Flips make all of this so much harder to think about.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+}<br>
+<br>
+static void<br>
+do_blorp_blit(struct blorp_batch *batch,<br>
+ struct blorp_params *params,<br>
+ struct brw_blorp_blit_prog_key *wm_prog_key,<br>
+ const struct blt_coords *orig)<br>
+{<br>
+ struct blt_coords split_coords = *orig;<br>
+ float w = orig->x.dst1 - orig->x.dst0;<br>
+ float h = orig->y.dst1 - orig->y.dst0;<br>
+ float x_scale = (orig->x.src1 - orig->x.src0) / w;<br>
+ float y_scale = (orig->y.src1 - orig->y.src0) / h;<br></blockquote><div><br></div><div>I've discovered in the past that these calculations can be very sensitive to precision. I'd recommend doubles instead of floats. See also fa4627149dfe7cdb9f75d8e2f1bcaf1ad7006801<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ if (orig->x.mirror)<br>
+ x_scale = -x_scale;<br>
+ if (orig->y.mirror)<br>
+ y_scale = -y_scale;<br>
+<br>
+ bool x_done, y_done;<br>
+ do {<br>
+ unsigned result =<br>
+ try_blorp_blit(batch, params, wm_prog_key, &split_coords);<br>
+<br>
+ if (result & BLIT_WIDTH_TOO_LARGE) {<br>
+ w /= 2.0;<br>
+ assert(w >= 1.0);<br>
+ split_coords.x.dst1 = MIN2(split_coords.x.dst0 + w, orig->x.dst1);<br>
+ adjust_split_coords(&orig->x, &split_coords.x, x_scale);<br>
+ }<br>
+ if (result & BLIT_HEIGHT_TOO_LARGE) {<br>
+ h /= 2.0;<br>
+ assert(h >= 1.0);<br>
+ split_coords.y.dst1 = MIN2(split_coords.y.dst0 + h, orig->y.dst1);<br>
+ adjust_split_coords(&orig->y, &split_coords.y, y_scale);<br>
+ }<br>
+<br>
+ if (result != 0)<br>
+ continue;<br>
+<br>
+ y_done = (orig->y.dst1 - split_coords.y.dst1 < 0.5);<br>
+ x_done = y_done && (orig->x.dst1 - split_coords.x.dst1 < 0.5);<br>
+ if (x_done) {<br>
+ break;<br>
+ } else if (y_done) {<br>
+ split_coords.x.dst0 += w;<br>
+ split_coords.x.dst1 = MIN2(split_coords.x.dst0 + w, orig->x.dst1);<br>
+ split_coords.y.dst0 = orig->y.dst0;<br>
+ adjust_split_coords(&orig->x, &split_coords.x, x_scale);<br>
+ } else {<br>
+ split_coords.y.dst0 += h;<br>
+ split_coords.y.dst1 = MIN2(split_coords.y.dst0 + h, orig->y.dst1);<br>
+ adjust_split_coords(&orig->y, &split_coords.y, y_scale);<br>
+ }<br>
+ } while (true);<br></blockquote><div><br></div><div>I applaud you for figuring out how to do this with a simple loop! I would have probably been lazy and written it recursively:<br><br></div><div>do_blorp_blit(batch, params, key, orig, splits)<br>{<br></div><div> if (splits & BLIT_WIDTH_TOO_LARGE) {<br></div><div> /* Adjust for left half */<br></div><div> do_blorp_blit(batch, params, key, orig, splits & ~BLIT_WIDTH_TOO_LARGE);<br></div><div> /* Adjust for right half */<br> do_blorp_blit(batch, params, key, orig, splits & ~BLIT_WIDTH_TOO_LARGE);<br></div><div> } else if (splits & BLIT_HEIGHT_TOO_LARGE) {<br></div><div> /* Adjust for top half */<br><div> do_blorp_blit(batch, params, key, orig, 0);<br></div> /* Adjust for bottom half */<br></div><div> do_blorp_blit(batch, params, key, orig, 0);<br></div><div> } else {<br></div><div> assert(splits == 0);<br></div><div> splits = try_blorp_blit();<br></div><div> if (splits)<br></div><div> do_blorp_blit(batch, params, key, orig, splits);<br></div><div> }<br></div><div>}<br><br>I find that it's easier to reason about these sorts of things with recursion. It just seems like it'd be easier to do the calculations if you were only ever cutting things in half. Take it or leave it. I believe your code is correct modulo some confusion around adjust_split_coords().<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
}<br>
<br>
void<br>
<span class="gmail-HOEnZb"><font color="#888888">--<br>
2.10.2<br>
<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div></div>