Re: [Mesa-dev] [PATCH v02 28/37] i965: Port gen6+ 3DSTATE_VS to genxml.

2017-04-27 Thread Kenneth Graunke 
On Wednesday, April 26, 2017 11:44:00 PM PDT Kenneth Graunke wrote:
> On Monday, April 24, 2017 3:19:23 PM PDT Rafael Antognolli wrote:
[snip]
> > +  vs.UserClipDistanceClipTestEnableBitmask = 
> > transform->ClipPlanesEnabled;
> > +  vs.UserClipDistanceCullTestEnableBitmask =
> > + vue_prog_data->cull_distance_mask;
> 
> We should drop the vs.UserClipDistance stuff - I removed it in commit
> 903056e016e3ea52c2f493f8b0938b519ee40894.

Sorry - misspoke.  We should drop ClipTestEnableBitmask.
We should keep CullTestEnableBitmask.

--Ken


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v02 28/37] i965: Port gen6+ 3DSTATE_VS to genxml.

2017-04-27 Thread Kenneth Graunke 
On Monday, April 24, 2017 3:19:23 PM PDT Rafael Antognolli wrote:
[snip]
> diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c 
> b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> index 17b8118..b2d2306 100644
> --- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> @@ -68,116 +68,3 @@ const struct brw_tracked_state gen6_vs_push_constants = {
> },
> .emit = gen6_upload_vs_push_constants,
>  };
> -
> -static void
> -upload_vs_state(struct brw_context *brw)
> -{
> -   const struct gen_device_info *devinfo = >screen->devinfo;
> -   const struct brw_stage_state *stage_state = >vs.base;
> -   const struct brw_stage_prog_data *prog_data = stage_state->prog_data;
> -   const struct brw_vue_prog_data *vue_prog_data =
> -  brw_vue_prog_data(stage_state->prog_data);
> -   uint32_t floating_point_mode = 0;
> -
> -   /* From the BSpec, 3D Pipeline > Geometry > Vertex Shader > State,
> -* 3DSTATE_VS, Dword 5.0 "VS Function Enable":
> -*
> -*   [DevSNB] A pipeline flush must be programmed prior to a 3DSTATE_VS
> -*   command that causes the VS Function Enable to toggle. Pipeline
> -*   flush can be executed by sending a PIPE_CONTROL command with CS
> -*   stall bit set and a post sync operation.
> -*
> -* We've already done such a flush at the start of state upload, so we
> -* don't need to do another one here.
> -*/

Please don't lose this comment.  Otherwise it looks like we've merely
forgotten to do the flush, and it's not missing intentionally.

> -
> -   if (stage_state->push_const_size == 0) {
> -  /* Disable the push constant buffers. */
> -  BEGIN_BATCH(5);
> -  OUT_BATCH(_3DSTATE_CONSTANT_VS << 16 | (5 - 2));
> -  OUT_BATCH(0);
> -  OUT_BATCH(0);
> -  OUT_BATCH(0);
> -  OUT_BATCH(0);
> -  ADVANCE_BATCH();
> -   } else {
> -  BEGIN_BATCH(5);
> -  OUT_BATCH(_3DSTATE_CONSTANT_VS << 16 |
> - GEN6_CONSTANT_BUFFER_0_ENABLE |
> - (5 - 2));
> -  /* Pointer to the VS constant buffer.  Covered by the set of
> -   * state flags from gen6_upload_vs_constants
> -   */
> -  OUT_BATCH(stage_state->push_const_offset +
> - stage_state->push_const_size - 1);
> -  OUT_BATCH(0);
> -  OUT_BATCH(0);
> -  OUT_BATCH(0);
> -  ADVANCE_BATCH();
> -   }
> -
> -   if (prog_data->use_alt_mode)
> -  floating_point_mode = GEN6_VS_FLOATING_POINT_MODE_ALT;
> -
> -   BEGIN_BATCH(6);
> -   OUT_BATCH(_3DSTATE_VS << 16 | (6 - 2));
> -   OUT_BATCH(stage_state->prog_offset);
> -   OUT_BATCH(floating_point_mode |
> -  ((ALIGN(stage_state->sampler_count, 4)/4) << 
> GEN6_VS_SAMPLER_COUNT_SHIFT) |
> - ((prog_data->binding_table.size_bytes / 4) <<
> -  GEN6_VS_BINDING_TABLE_ENTRY_COUNT_SHIFT));
> -
> -   if (prog_data->total_scratch) {
> -  OUT_RELOC(stage_state->scratch_bo,
> - I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
> - ffs(stage_state->per_thread_scratch) - 11);
> -   } else {
> -  OUT_BATCH(0);
> -   }
> -
> -   OUT_BATCH((prog_data->dispatch_grf_start_reg <<
> -  GEN6_VS_DISPATCH_START_GRF_SHIFT) |
> -  (vue_prog_data->urb_read_length << GEN6_VS_URB_READ_LENGTH_SHIFT) |
> -  (0 << GEN6_VS_URB_ENTRY_READ_OFFSET_SHIFT));
> -
> -   OUT_BATCH(((devinfo->max_vs_threads - 1) << GEN6_VS_MAX_THREADS_SHIFT) |
> -  GEN6_VS_STATISTICS_ENABLE |
> -  GEN6_VS_ENABLE);
> -   ADVANCE_BATCH();
> -
> -   /* Based on my reading of the simulator, the VS constants don't get
> -* pulled into the VS FF unit until an appropriate pipeline flush
> -* happens, and instead the 3DSTATE_CONSTANT_VS packet just adds
> -* references to them into a little FIFO.  The flushes are common,
> -* but don't reliably happen between this and a 3DPRIMITIVE, causing
> -* the primitive to use the wrong constants.  Then the FIFO
> -* containing the constant setup gets added to again on the next
> -* constants change, and eventually when a flush does happen the
> -* unit is overwhelmed by constant changes and dies.
> -*
> -* To avoid this, send a PIPE_CONTROL down the line that will
> -* update the unit immediately loading the constants.  The flush
> -* type bits here were those set by the STATE_BASE_ADDRESS whose
> -* move in a82a43e8d99e1715dd11c9c091b5ab734079b6a6 triggered the
> -* bug reports that led to this workaround, and may be more than
> -* what is strictly required to avoid the issue.
> -*/

Please don't lose this comment.  The reason for the flush is not at all
obvious, and without the explanation, someone might try and remove it.

> -   brw_emit_pipe_control_flush(brw,
> -   PIPE_CONTROL_DEPTH_STALL |
> -   PIPE_CONTROL_INSTRUCTION_INVALIDATE |
> -   PIPE_CONTROL_STATE_CACHE_INVALIDATE);
> -}
> -
> -const struct

[Mesa-dev] [PATCH v02 28/37] i965: Port gen6+ 3DSTATE_VS to genxml.

2017-04-24 Thread Rafael Antognolli 
Emit 3DSTATE_VS on Gen6+ using brw_batch_emit helper, that uses pack
structs from genxml.

v2:
   - Use render_bo helper to setup brw_address (Kristian)

Signed-off-by: Rafael Antognolli 
---
 src/mesa/drivers/dri/i965/Makefile.sources|   2 +-
 src/mesa/drivers/dri/i965/brw_state.h |   3 +-
 src/mesa/drivers/dri/i965/gen6_vs_state.c | 113 +---
 src/mesa/drivers/dri/i965/gen7_vs_state.c |  87 +---
 src/mesa/drivers/dri/i965/gen8_vs_state.c |  96 +
 src/mesa/drivers/dri/i965/genX_state_upload.c | 106 +-
 6 files changed, 103 insertions(+), 304 deletions(-)
 delete mode 100644 src/mesa/drivers/dri/i965/gen7_vs_state.c
 delete mode 100644 src/mesa/drivers/dri/i965/gen8_vs_state.c

diff --git a/src/mesa/drivers/dri/i965/Makefile.sources 
b/src/mesa/drivers/dri/i965/Makefile.sources
index 7f25ae1..95d29ac 100644
--- a/src/mesa/drivers/dri/i965/Makefile.sources
+++ b/src/mesa/drivers/dri/i965/Makefile.sources
@@ -100,7 +100,6 @@ i965_FILES = \
gen7_te_state.c \
gen7_urb.c \
gen7_viewport_state.c \
-   gen7_vs_state.c \
gen7_wm_surface_state.c \
gen8_blend_state.c \
gen8_depth_state.c \
@@ -111,7 +110,6 @@ i965_FILES = \
gen8_multisample_state.c \
gen8_surface_state.c \
gen8_viewport_state.c \
-   gen8_vs_state.c \
hsw_queryobj.c \
hsw_sol.c \
intel_batchbuffer.c \
diff --git a/src/mesa/drivers/dri/i965/brw_state.h 
b/src/mesa/drivers/dri/i965/brw_state.h
index a87bf3a..72d63f6 100644
--- a/src/mesa/drivers/dri/i965/brw_state.h
+++ b/src/mesa/drivers/dri/i965/brw_state.h
@@ -123,7 +123,6 @@ extern const struct brw_tracked_state gen6_sf_vp;
 extern const struct brw_tracked_state gen6_urb;
 extern const struct brw_tracked_state gen6_viewport_state;
 extern const struct brw_tracked_state gen6_vs_push_constants;
-extern const struct brw_tracked_state gen6_vs_state;
 extern const struct brw_tracked_state gen6_wm_push_constants;
 extern const struct brw_tracked_state gen7_depthbuffer;
 extern const struct brw_tracked_state gen7_ds_state;
@@ -136,7 +135,6 @@ extern const struct brw_tracked_state gen7_sf_clip_viewport;
 extern const struct brw_tracked_state gen7_te_state;
 extern const struct brw_tracked_state gen7_tes_push_constants;
 extern const struct brw_tracked_state gen7_urb;
-extern const struct brw_tracked_state gen7_vs_state;
 extern const struct brw_tracked_state haswell_cut_index;
 extern const struct brw_tracked_state gen8_blend_state;
 extern const struct brw_tracked_state gen8_ds_state;
@@ -149,7 +147,6 @@ extern const struct brw_tracked_state gen8_ps_blend;
 extern const struct brw_tracked_state gen8_sf_clip_viewport;
 extern const struct brw_tracked_state gen8_vertices;
 extern const struct brw_tracked_state gen8_vf_topology;
-extern const struct brw_tracked_state gen8_vs_state;
 extern const struct brw_tracked_state brw_cs_work_groups_surface;
 
 static inline bool
diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c 
b/src/mesa/drivers/dri/i965/gen6_vs_state.c
index 17b8118..b2d2306 100644
--- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
@@ -68,116 +68,3 @@ const struct brw_tracked_state gen6_vs_push_constants = {
},
.emit = gen6_upload_vs_push_constants,
 };
-
-static void
-upload_vs_state(struct brw_context *brw)
-{
-   const struct gen_device_info *devinfo = >screen->devinfo;
-   const struct brw_stage_state *stage_state = >vs.base;
-   const struct brw_stage_prog_data *prog_data = stage_state->prog_data;
-   const struct brw_vue_prog_data *vue_prog_data =
-  brw_vue_prog_data(stage_state->prog_data);
-   uint32_t floating_point_mode = 0;
-
-   /* From the BSpec, 3D Pipeline > Geometry > Vertex Shader > State,
-* 3DSTATE_VS, Dword 5.0 "VS Function Enable":
-*
-*   [DevSNB] A pipeline flush must be programmed prior to a 3DSTATE_VS
-*   command that causes the VS Function Enable to toggle. Pipeline
-*   flush can be executed by sending a PIPE_CONTROL command with CS
-*   stall bit set and a post sync operation.
-*
-* We've already done such a flush at the start of state upload, so we
-* don't need to do another one here.
-*/
-
-   if (stage_state->push_const_size == 0) {
-  /* Disable the push constant buffers. */
-  BEGIN_BATCH(5);
-  OUT_BATCH(_3DSTATE_CONSTANT_VS << 16 | (5 - 2));
-  OUT_BATCH(0);
-  OUT_BATCH(0);
-  OUT_BATCH(0);
-  OUT_BATCH(0);
-  ADVANCE_BATCH();
-   } else {
-  BEGIN_BATCH(5);
-  OUT_BATCH(_3DSTATE_CONSTANT_VS << 16 |
-   GEN6_CONSTANT_BUFFER_0_ENABLE |
-   (5 - 2));
-  /* Pointer to the VS constant buffer.  Covered by the set of
-   * state flags from gen6_upload_vs_constants
-   */
-  OUT_BATCH(stage_state->push_const_offset +
-   stage_state->push_const_size - 1);
-