Can someone review this patch? The bug if fixes is blocking the 18.0 release.
Iago On Tue, 2018-02-27 at 08:02 +0100, Iago Toral Quiroga wrote: > 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). > > v2: > - Add 'Fixes' tag (Mark Janes) > - make no_vue_inputs int instead of uint32_t, and add const > qualifier > to num_inputs variable (Ian) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105224 > Fixes: 16631ca30ea6 (i965/sbe: fix active components for SSO programs > with over 16 inputs) > --- > 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..891bab1746 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); > > + int 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); > + const 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; > } _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
