[Mesa-dev] [PATCH 1/2] i965: Refactor brw_draw_prims.

2017-08-29 Thread Plamena Manolova
brw_draw_prims needs to be refactored prior to
ARB_indirect_parameters implementation.

Signed-off-by: Plamena Manolova 
---
 src/mesa/drivers/dri/i965/brw_draw.c | 343 +++
 1 file changed, 189 insertions(+), 154 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_draw.c 
b/src/mesa/drivers/dri/i965/brw_draw.c
index a8ad2ac..7597bae 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -531,7 +531,7 @@ brw_postdraw_set_buffers_need_resolve(struct brw_context 
*brw)
 
   if (!irb)
  continue;
- 
+
   brw_render_cache_set_add_bo(brw, irb->mt->bo);
   intel_miptree_finish_render(brw, irb->mt, irb->mt_level,
   irb->mt_layer, irb->layer_count,
@@ -594,21 +594,163 @@ brw_postdraw_reconcile_align_wa_slices(struct 
brw_context *brw)
  * fallback conditions.
  */
 static void
-brw_try_draw_prims(struct gl_context *ctx,
-   const struct gl_vertex_array *arrays[],
-   const struct _mesa_prim *prims,
-   GLuint nr_prims,
-   const struct _mesa_index_buffer *ib,
-   bool index_bounds_valid,
-   GLuint min_index,
-   GLuint max_index,
-   struct brw_transform_feedback_object *xfb_obj,
-   unsigned stream,
-   struct gl_buffer_object *indirect)
+brw_try_draw_prim(struct gl_context *ctx,
+  const struct gl_vertex_array *arrays[],
+  const struct _mesa_prim *prim,
+  const struct _mesa_index_buffer *ib,
+  bool index_bounds_valid,
+  GLuint min_index,
+  GLuint max_index,
+  struct brw_transform_feedback_object *xfb_obj,
+  unsigned stream,
+  struct gl_buffer_object *indirect)
 {
struct brw_context *brw = brw_context(ctx);
-   GLuint i;
bool fail_next = false;
+   int estimated_max_prim_size;
+   const int sampler_state_size = 16;
+
+   estimated_max_prim_size = 512; /* batchbuffer commands */
+   estimated_max_prim_size += BRW_MAX_TEX_UNIT *
+  (sampler_state_size + sizeof(struct gen5_sampler_default_color));
+   estimated_max_prim_size += 1024; /* gen6 VS push constants */
+   estimated_max_prim_size += 1024; /* gen6 WM push constants */
+   estimated_max_prim_size += 512; /* misc. pad */
+
+   /* Flag BRW_NEW_DRAW_CALL on every draw.  This allows us to have
+* atoms that happen on every draw call.
+*/
+   brw->ctx.NewDriverState |= BRW_NEW_DRAW_CALL;
+
+   /* Flush the batch if it's approaching full, so that we don't wrap while
+* we've got validated state that needs to be in the same batch as the
+* primitives.
+*/
+   intel_batchbuffer_require_space(brw, estimated_max_prim_size, RENDER_RING);
+   intel_batchbuffer_save_state(brw);
+
+   if (brw->num_instances != prim->num_instances ||
+   brw->basevertex != prim->basevertex ||
+   brw->baseinstance != prim->base_instance) {
+   brw->num_instances = prim->num_instances;
+   brw->basevertex = prim->basevertex;
+   brw->baseinstance = prim->base_instance;
+  if (prim->draw_id > 0) { /* For draw_id == 0 we just did this before the 
loop */
+ brw->ctx.NewDriverState |= BRW_NEW_VERTICES;
+ brw_merge_inputs(brw, arrays);
+  }
+   }
+
+   /* Determine if we need to flag BRW_NEW_VERTICES for updating the
+* gl_BaseVertexARB or gl_BaseInstanceARB values. For indirect draw, we
+* always flag if the shader uses one of the values. For direct draws,
+* we only flag if the values change.
+*/
+   const int new_basevertex =
+  prim->indexed ? prim->basevertex : prim->start;
+   const int new_baseinstance = prim->base_instance;
+   const struct brw_vs_prog_data *vs_prog_data =
+  brw_vs_prog_data(brw->vs.base.prog_data);
+   if (prim->draw_id > 0) {
+  const bool uses_draw_parameters =
+ vs_prog_data->uses_basevertex ||
+ vs_prog_data->uses_baseinstance;
+
+  if ((uses_draw_parameters && prim->is_indirect) ||
+  (vs_prog_data->uses_basevertex &&
+   brw->draw.params.gl_basevertex != new_basevertex) ||
+  (vs_prog_data->uses_baseinstance &&
+   brw->draw.params.gl_baseinstance != new_baseinstance))
+ brw->ctx.NewDriverState |= BRW_NEW_VERTICES;
+   }
+
+   brw->draw.params.gl_basevertex = new_basevertex;
+   brw->draw.params.gl_baseinstance = new_baseinstance;
+   brw_bo_unreference(brw->draw.draw_params_bo);
+
+   if (prim->is_indirect) {
+  /* Point draw_params_bo at the indirect buffer. */
+  brw->draw.draw_params_bo =
+  intel_buffer_object(ctx->DrawIndirectBuffer)->buffer;
+  brw_bo_reference(brw->draw.draw_params_bo);
+  brw->draw.draw_params_offset =
+  prim->indirect_offset + (prim->indexed ? 12 : 8);
+   } else {
+  /* Set draw_params_bo to NULL so brw_prep

Re: [Mesa-dev] [PATCH 1/2] i965: Refactor brw_draw_prims.

2017-09-25 Thread Kenneth Graunke
On Tuesday, August 29, 2017 9:24:00 AM PDT Plamena Manolova wrote:
> brw_draw_prims needs to be refactored prior to
> ARB_indirect_parameters implementation.
> 
> Signed-off-by: Plamena Manolova 
> ---
>  src/mesa/drivers/dri/i965/brw_draw.c | 343 
> +++
>  1 file changed, 189 insertions(+), 154 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c 
> b/src/mesa/drivers/dri/i965/brw_draw.c
> index a8ad2ac..7597bae 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -531,7 +531,7 @@ brw_postdraw_set_buffers_need_resolve(struct brw_context 
> *brw)
>  
>if (!irb)
>   continue;
> - 
> +
>brw_render_cache_set_add_bo(brw, irb->mt->bo);
>intel_miptree_finish_render(brw, irb->mt, irb->mt_level,
>irb->mt_layer, irb->layer_count,
> @@ -594,21 +594,163 @@ brw_postdraw_reconcile_align_wa_slices(struct 
> brw_context *brw)
>   * fallback conditions.
>   */
>  static void
> -brw_try_draw_prims(struct gl_context *ctx,
> -   const struct gl_vertex_array *arrays[],
> -   const struct _mesa_prim *prims,
> -   GLuint nr_prims,
> -   const struct _mesa_index_buffer *ib,
> -   bool index_bounds_valid,
> -   GLuint min_index,
> -   GLuint max_index,
> -   struct brw_transform_feedback_object *xfb_obj,
> -   unsigned stream,
> -   struct gl_buffer_object *indirect)
> +brw_try_draw_prim(struct gl_context *ctx,
> +  const struct gl_vertex_array *arrays[],
> +  const struct _mesa_prim *prim,
> +  const struct _mesa_index_buffer *ib,
> +  bool index_bounds_valid,
> +  GLuint min_index,
> +  GLuint max_index,
> +  struct brw_transform_feedback_object *xfb_obj,
> +  unsigned stream,
> +  struct gl_buffer_object *indirect)
>  {

Hey Pam,

I'm sorry it took me so long to look at these patches...especially since
this refactoring patch isn't going to apply anymore. :(

A couple of bits of feedback:

- You changed 'i' to 'prim->draw_id' in a couple of places.  That looks
  fine, but could we do that in a separate patch, before the other
  refactors?  That way, if the vbo module fails to set prim->draw_id in
  some case, and this breaks something, 'git bisect' would find a tiny
  simple patch instead of one with a lot of code motion.

- What do you think of calling these functions...

  - brw_prepare_drawing()  (instead of brw_prepare_draw_prims)
  - brw_finish_drawing()   (instead of brw_end_draw_prims)
  - brw_draw_single_prim() (instead of brw_try_draw_prim)

- Would it be possible to introduce those functions in separate patches?

Otherwise, this looks good to me.

I'll try and land your new refactoring patches as soon as you respin
them so you don't end up having to rewrite them a third time.  Again,
sorry about that...!

--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