[Mesa-dev] [PATCH] gallium/util: handle patches in u_prims_for_vertices to fix a radeonsi crash

[eocallaghan at alterapraxis.com]

On 2015-12-10 01:47, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> I guess the crash was because of divison by zero.
> 
> Cc: 11.0 11.1 <mesa-stable at lists.freedesktop.org>
> ---
>  src/gallium/auxiliary/util/u_prim.h          | 17 +++++++++++++----
>  src/gallium/drivers/radeonsi/si_state_draw.c |  3 ++-
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/util/u_prim.h
> b/src/gallium/auxiliary/util/u_prim.h
> index 3668015..4926af6 100644
> --- a/src/gallium/auxiliary/util/u_prim.h
> +++ b/src/gallium/auxiliary/util/u_prim.h
> @@ -141,14 +141,23 @@ u_prim_vertex_count(unsigned prim)
>   * For polygons, return the number of triangles.
>   */
>  static inline unsigned
> -u_prims_for_vertices(unsigned prim, unsigned num)
> +u_prims_for_vertices(unsigned prim, unsigned num, unsigned 
> vertices_per_patch)
>  {
> -   const struct u_prim_vertex_count *info = u_prim_vertex_count(prim);
> +   struct u_prim_vertex_count info;
> 
> -   if (num < info->min)
> +   if (prim == PIPE_PRIM_PATCHES)
> +      info.min = info.incr = vertices_per_patch;
> +   else if (prim < PIPE_PRIM_MAX)

We already do this check in u_prim_vertex_count() and if out-of-bounds 
we
returned a NULL. Perhaps it would be better avoid this extra else-if 
branch
here and just in the else branch, make the call and then assert on the 
NULL.

> +      info = *u_prim_vertex_count(prim);
> +   else {
> +      assert(!"invalid prim type");
> +      return 0;
> +   }
> +
> +   if (num < info.min)
>        return 0;

Well convolving this with my previous patch,
http://lists.freedesktop.org/archives/mesa-dev/2015-December/102729.html
I think we should still have an  assert(info.incr != 0);  here.

> 
> -   return 1 + ((num - info->min) / info->incr);
> +   return 1 + ((num - info.min) / info.incr);
>  }
> 
>  static inline boolean u_validate_pipe_prim( unsigned pipe_prim, 
> unsigned nr )
> diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c
> b/src/gallium/drivers/radeonsi/si_state_draw.c
> index ee84a1f..4ac9d0a 100644
> --- a/src/gallium/drivers/radeonsi/si_state_draw.c
> +++ b/src/gallium/drivers/radeonsi/si_state_draw.c
> @@ -320,7 +320,8 @@ static unsigned si_get_ia_multi_vgt_param(struct
> si_context *sctx,
>  	if (sctx->b.screen->info.max_se >= 2 && ia_switch_on_eoi &&
>  	    (info->indirect ||
>  	     (info->instance_count > 1 &&
> -	      u_prims_for_vertices(info->mode, info->count) <= 1)))
> +	      u_prims_for_vertices(info->mode, info->count,
> +				   info->vertices_per_patch) <= 1)))
>  		sctx->b.flags |= SI_CONTEXT_VGT_FLUSH;
> 
>  	return S_028AA8_SWITCH_ON_EOP(ia_switch_on_eop) |