Attachment 137603 Details for Bug 105224 – Fix

[patch] Fix

0001-i965-sbe-fix-number-of-inputs-for-active-components.patch (text/plain), 4.54 KB, created by Iago Toral on 2018-02-26 09:48:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Iago Toral

Created: 2018-02-26 09:48:16 UTC

Size: 4.54 KB

patch

obsolete

>From ea293b932ff3ac1f2c8111770eede1b5afc45aba Mon Sep 17 00:00:00 2001
>From: Iago Toral Quiroga <itoral@igalia.com>
>Date: Mon, 26 Feb 2018 10:02:13 +0100
>Subject: [PATCH] i965/sbe: fix number of inputs for active components
>
>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
>

Actions: View | Splinter Review

Attachments on bug 105224: 137555 | 137561 | 137565 | 137566 | 137567 | 137568 | 137603

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.