[Mesa-dev] [v2 1/2] i965: Validate textures before altering driver state

[Ian Romanick]

On 01/28/2016 02:29 PM, Topi Pohjolainen wrote:
> Validation may kick off copies and subsequently color resolves.
> Color resolves (and the copies themselves if ending up in meta path)
> will overwrite the internal driver state but are not prepared to
> restore it. Instead of adding that capability the validation can be
> simply performed before the state is updated.

The problem isn't src/mesa/drivers/common/meta*.  The problem is
brw_meta_* because it directly pokes at driver context state instead of
using higher-level operations, right?

Do we have a test case that encounters this problem?  It seems like this
must have been very difficult to detect, and that worries me.  It's
completely non-obvious that brw_validate_textures will do meta ops.  I
wonder if we could add some sort of flag in debug builds to signal
"doing meta would be bad right now."

Either way, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

After it gets a bit of extra soak time on master, we may want to
nominate it for stable.  This clearly fixes a problem, and I don't want
to have to debug an application rendering error that could result from
it. :)

> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_draw.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
> index 8737c64..c295d91 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -420,6 +420,15 @@ brw_try_draw_prims(struct gl_context *ctx,
>     if (ctx->NewState)
>        _mesa_update_state(ctx);
>  
> +   /* We have to validate the textures *before* checking for fallbacks;
> +    * otherwise, the software fallback won't be able to rely on the
> +    * texture state, the firstLevel and lastLevel fields won't be
> +    * set in the intel texture object (they'll both be 0), and the
> +    * software fallback will segfault if it attempts to access any
> +    * texture level other than level 0.
> +    */
> +   brw_validate_textures(brw);
> +
>     /* Find the highest sampler unit used by each shader program.  A bit-count
>      * won't work since ARB programs use the texture unit number as the sampler
>      * index.
> @@ -435,15 +444,6 @@ brw_try_draw_prims(struct gl_context *ctx,
>     brw->vs.base.sampler_count =
>        _mesa_fls(ctx->VertexProgram._Current->Base.SamplersUsed);
>  
> -   /* We have to validate the textures *before* checking for fallbacks;
> -    * otherwise, the software fallback won't be able to rely on the
> -    * texture state, the firstLevel and lastLevel fields won't be
> -    * set in the intel texture object (they'll both be 0), and the
> -    * software fallback will segfault if it attempts to access any
> -    * texture level other than level 0.
> -    */
> -   brw_validate_textures(brw);
> -
>     intel_prepare_render(brw);
>  
>     /* This workaround has to happen outside of brw_upload_render_state()
>