[Mesa-dev] [RFC] i965: Restore vbo after color resolve during brw_try_draw_prims()

[Topi Pohjolainen]

Part of brw_try_draw_prims() is a check to validate textures
(brw_validate_textures()). In case of textures that currently have
only level zero but are marked for mipmap generation, i965 driver
will decide to replace the underlying buffer with a larger one
capable of holding also the additional levels. This results into
blit from the original buffer to the newly allocated (see
intel_miptree_copy_teximage()). This blit is currently handled with
blitter engine and hence it won't effect the ongoing draw operation.
However, this blit in turn may trigger color resolve on the source
buffer. In principle, this should be possible with fast cleared
buffers but I only started hitting it when I enabled lossless
compression (that reguires similar resolve to fast cleared buffers).

Now, the color resolve is a meta operation and uses the same drawing
path we are already in middle of. After quite a bit of debugging I
realized that the resolve will modify the current vbo setup but it
won't restore it afterwards resulting in the original draw call
using wrong vertex data.
When brw_try_draw_prims() gets called, the vbo logic in the Mesa
core (see vbo_draw_arrays()) has just bound the vbo (see
vbo_bind_arrays() and recalculate_input_bindings()). Color resolve
operation will overwrite the vbo setup by calling vbo_bind_arrays()
against the resolve rectangle (see brw_draw_rectlist()). Once the
color resolve is done the vbo setup is left to the resolve rectangle
state and the original drawing call yields bogus results.

This patch aims to restore the original state after the color
resolve by calling vbo_bind_arrays() yet again after the vertex
array state in the core context have been restored.

Now having said all this, I'd also like to state that I'm quite
uncomfortable with the nested meta operations. Ths original draw
call in this case is in fact a meta operation itself. It is a blit
from level zero to level one when generating the additional mipmap
levels (see _mesa_meta_GenerateMipmap()). Imagine the complexity
if the blit in the middle from buffer to another would go to meta
path also instead of blitter.

I would very tempted to try to move all the resolves to happen
before a meta operation is started.
Additionally I still feel that work I did earlier in the spring/
summer time moving meta operations to use direct state upload
bypassing the core context would make sense.

Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
---
 src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
index d338659..affe1bc 100644
--- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
+++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
@@ -896,6 +896,9 @@ brw_meta_resolve_color(struct brw_context *brw,
 
    _mesa_meta_end(ctx);
 
+   /* Restore in case we were called in the middle of brw_try_draw_prims() */
+   vbo_bind_arrays(ctx);
+
    /* We're typically called from intel_update_state() and we're supposed to
     * return with the state all updated to what it was before
     * brw_meta_resolve_color() was called.  The meta rendering will have
-- 
2.5.0