[Mesa-dev] [PATCH] i965/sbe: fix number of inputs for active components

[Mark Janes]

Please add the Fixes: tag, so this patch gets applied to the appropriate
stable branches.

Iago Toral Quiroga <itoral at igalia.com> writes:

> In 16631ca30ea6 we fixed gen9 active components to account for padded
> inputs in the URB, which we can have with SSO programs. To do that,
> instead of going through the bitfield of inputs (which doesn't include
> padding information), we compute the number of inputs from the size
> of the URB entry.
>
> Unfortunately, there are some special inputs that are not stored in
> the URB and that we also need to account for. These special inputs
> are identified and handled during calculate_attr_overrides(), so this
> patch modifies this function to return a value with the total number
> of inputs, including the ones that are not stored in the URB, so we
> can use that number to program the correct number of active components.
>
> This fixes a regression in a WebGL program that uses Point Sprite
> functionality (specifically, VARYING_SLOT_PNTC).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105224
> ---
>  src/mesa/drivers/dri/i965/genX_state_upload.c | 31 ++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index 8668abd591..617f0ab2c4 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -1015,7 +1015,7 @@ genX(get_attr_override)(struct GENX(SF_OUTPUT_ATTRIBUTE_DETAIL) *attr,
>  }
>  
>  
> -static void
> +static int
>  genX(calculate_attr_overrides)(const struct brw_context *brw,
>                                 struct GENX(SF_OUTPUT_ATTRIBUTE_DETAIL) *attr_overrides,
>                                 uint32_t *point_sprite_enables,
> @@ -1064,6 +1064,7 @@ genX(calculate_attr_overrides)(const struct brw_context *brw,
>      */
>     bool drawing_points = brw_is_drawing_points(brw);
>  
> +   uint32_t no_vue_inputs = 0;
>     for (int attr = 0; attr < VARYING_SLOT_MAX; attr++) {
>        int input_index = wm_prog_data->urb_setup[attr];
>  
> @@ -1097,6 +1098,12 @@ genX(calculate_attr_overrides)(const struct brw_context *brw,
>                                   &max_source_attr);
>        }
>  
> +      if (point_sprite ||
> +          (attr == VARYING_SLOT_PRIMITIVE_ID &&
> +           brw->vue_map_geom_out.varying_to_slot[attr] == -1)) {
> +         no_vue_inputs++;
> +      }
> +
>        /* The hardware can only do the overrides on 16 overrides at a
>         * time, and the other up to 16 have to be lined up so that the
>         * input index = the output index.  We'll need to do some
> @@ -1124,6 +1131,8 @@ genX(calculate_attr_overrides)(const struct brw_context *brw,
>      * Similar text exists for Ivy Bridge.
>      */
>     *urb_entry_read_length = DIV_ROUND_UP(max_source_attr + 1, 2);
> +
> +   return *urb_entry_read_length * 2 + no_vue_inputs;
>  }
>  #endif
>  
> @@ -3434,11 +3443,12 @@ genX(upload_sbe)(struct brw_context *brw)
>         * BRW_NEW_GS_PROG_DATA | BRW_NEW_PRIMITIVE | BRW_NEW_TES_PROG_DATA |
>         * BRW_NEW_VUE_MAP_GEOM_OUT
>         */
> -      genX(calculate_attr_overrides)(brw,
> -                                     attr_overrides,
> -                                     &point_sprite_enables,
> -                                     &urb_entry_read_length,
> -                                     &urb_entry_read_offset);
> +      int num_inputs =
> +         genX(calculate_attr_overrides)(brw,
> +                                        attr_overrides,
> +                                        &point_sprite_enables,
> +                                        &urb_entry_read_length,
> +                                        &urb_entry_read_offset);
>  
>        /* Typically, the URB entry read length and offset should be programmed
>         * in 3DSTATE_VS and 3DSTATE_GS; SBE inherits it from the last active
> @@ -3459,8 +3469,13 @@ genX(upload_sbe)(struct brw_context *brw)
>  #endif
>  
>  #if GEN_GEN >= 9
> -      /* prepare the active component dwords */
> -      const int num_inputs = urb_entry_read_length * 2;
> +      /* prepare the active component dwords
> +       *
> +       * For this, we need to account for padded inputs (that we can have with
> +       * SSO programs), so we take the number of inputs from the size of the
> +       * URB entry. To that, we need to add the number of special inputs, if
> +       * any, that are not in the URB (such as point sprite inputs).
> +       */
>        for (int input_index = 0; input_index < num_inputs; input_index++) {
>           sbe.AttributeActiveComponentFormat[input_index] = ACTIVE_COMPONENT_XYZW;
>        }
> -- 
> 2.14.1