[Mesa-dev] [PATCH v02 19/37] i965: Port Gen6+ 3DSTATE_CLIP state to genxml.

Kenneth Graunke kenneth at whitecape.org
Tue Apr 25 23:35:29 UTC 2017


On Monday, April 24, 2017 3:19:14 PM PDT Rafael Antognolli wrote:
> Emit clip state on Gen6+ using brw_batch_emit helper, using pack structs
> from genxml.
> 
> Signed-off-by: Rafael Antognolli <rafael.antognolli at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_state.h         |   1 +-
>  src/mesa/drivers/dri/i965/gen6_clip_state.c   | 139 +------------------
>  src/mesa/drivers/dri/i965/genX_state_upload.c | 143 ++++++++++++++++++-
>  3 files changed, 140 insertions(+), 143 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
> index 7b6d718..c26be41 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -109,7 +109,6 @@ extern const struct brw_tracked_state brw_cs_state;
>  extern const struct brw_tracked_state gen7_cs_push_constants;
>  extern const struct brw_tracked_state gen6_binding_table_pointers;
>  extern const struct brw_tracked_state gen6_blend_state;
> -extern const struct brw_tracked_state gen6_clip_state;
>  extern const struct brw_tracked_state gen6_sf_and_clip_viewports;
>  extern const struct brw_tracked_state gen6_color_calc_state;
>  extern const struct brw_tracked_state gen6_gs_state;
> diff --git a/src/mesa/drivers/dri/i965/gen6_clip_state.c b/src/mesa/drivers/dri/i965/gen6_clip_state.c
> index 23d969b..2fffb67 100644
> --- a/src/mesa/drivers/dri/i965/gen6_clip_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_clip_state.c
> @@ -88,142 +88,3 @@ brw_is_drawing_lines(const struct brw_context *brw)
>     return false;
>  }
>  
> -static void
> -upload_clip_state(struct brw_context *brw)
> -{
> -   struct gl_context *ctx = &brw->ctx;
> -   /* BRW_NEW_META_IN_PROGRESS */
> -   uint32_t dw1 = brw->meta_in_progress ? 0 : GEN6_CLIP_STATISTICS_ENABLE;
> -   uint32_t dw2 = 0;
> -
> -   /* _NEW_BUFFERS */
> -   struct gl_framebuffer *fb = ctx->DrawBuffer;
> -
> -   /* BRW_NEW_FS_PROG_DATA */
> -   if (brw_wm_prog_data(brw->wm.base.prog_data)->barycentric_interp_modes &
> -       BRW_BARYCENTRIC_NONPERSPECTIVE_BITS) {
> -      dw2 |= GEN6_CLIP_NON_PERSPECTIVE_BARYCENTRIC_ENABLE;
> -   }
> -
> -   /* BRW_NEW_VS_PROG_DATA */
> -   dw1 |= brw_vue_prog_data(brw->vs.base.prog_data)->cull_distance_mask;
> -
> -   if (brw->gen >= 7)
> -      dw1 |= GEN7_CLIP_EARLY_CULL;
> -
> -   if (brw->gen == 7) {
> -      /* _NEW_POLYGON */
> -      if (ctx->Polygon._FrontBit == _mesa_is_user_fbo(fb))
> -         dw1 |= GEN7_CLIP_WINDING_CCW;
> -
> -      if (ctx->Polygon.CullFlag) {
> -         switch (ctx->Polygon.CullFaceMode) {
> -         case GL_FRONT:
> -            dw1 |= GEN7_CLIP_CULLMODE_FRONT;
> -            break;
> -         case GL_BACK:
> -            dw1 |= GEN7_CLIP_CULLMODE_BACK;
> -            break;
> -         case GL_FRONT_AND_BACK:
> -            dw1 |= GEN7_CLIP_CULLMODE_BOTH;
> -            break;
> -         default:
> -            unreachable("Should not get here: invalid CullFlag");
> -         }
> -      } else {
> -         dw1 |= GEN7_CLIP_CULLMODE_NONE;
> -      }
> -   }
> -
> -   if (brw->gen < 8 && !ctx->Transform.DepthClamp)
> -      dw2 |= GEN6_CLIP_Z_TEST;
> -
> -   /* _NEW_LIGHT */
> -   if (ctx->Light.ProvokingVertex == GL_FIRST_VERTEX_CONVENTION) {
> -      dw2 |=
> -	 (0 << GEN6_CLIP_TRI_PROVOKE_SHIFT) |
> -	 (1 << GEN6_CLIP_TRIFAN_PROVOKE_SHIFT) |
> -	 (0 << GEN6_CLIP_LINE_PROVOKE_SHIFT);
> -   } else {
> -      dw2 |=
> -	 (2 << GEN6_CLIP_TRI_PROVOKE_SHIFT) |
> -	 (2 << GEN6_CLIP_TRIFAN_PROVOKE_SHIFT) |
> -	 (1 << GEN6_CLIP_LINE_PROVOKE_SHIFT);
> -   }
> -
> -   /* _NEW_TRANSFORM */
> -   dw2 |= (ctx->Transform.ClipPlanesEnabled <<
> -           GEN6_USER_CLIP_CLIP_DISTANCES_SHIFT);
> -
> -   /* Have the hardware use the user clip distance clip test enable bitmask
> -    * specified here in 3DSTATE_CLIP rather than the one in 3DSTATE_VS/DS/GS.
> -    * We already listen to _NEW_TRANSFORM here, but the other atoms don't
> -    * need to other than this.
> -    */
> -   if (brw->gen >= 8)
> -      dw1 |= GEN8_CLIP_FORCE_USER_CLIP_DISTANCE_BITMASK;
> -
> -   if (ctx->Transform.ClipDepthMode == GL_ZERO_TO_ONE)
> -      dw2 |= GEN6_CLIP_API_D3D;
> -   else
> -      dw2 |= GEN6_CLIP_API_OGL;
> -
> -   dw2 |= GEN6_CLIP_GB_TEST;
> -
> -   /* BRW_NEW_VIEWPORT_COUNT */
> -   const unsigned viewport_count = brw->clip.viewport_count;
> -
> -   /* BRW_NEW_RASTERIZER_DISCARD */
> -   if (ctx->RasterDiscard) {
> -      dw2 |= GEN6_CLIP_MODE_REJECT_ALL;
> -      if (brw->gen == 6) {
> -         perf_debug("Rasterizer discard is currently implemented via the "
> -                    "clipper; having the GS not write primitives would "
> -                    "likely be faster.\n");
> -      }
> -   }
> -
> -   uint32_t enable;
> -   if (brw->primitive == _3DPRIM_RECTLIST)
> -      enable = 0;
> -   else
> -      enable = GEN6_CLIP_ENABLE;
> -
> -   /* _NEW_POLYGON,
> -    * BRW_NEW_GEOMETRY_PROGRAM | BRW_NEW_TES_PROG_DATA | BRW_NEW_PRIMITIVE
> -    */
> -   if (!brw_is_drawing_points(brw) && !brw_is_drawing_lines(brw))
> -      dw2 |= GEN6_CLIP_XY_TEST;
> -
> -   BEGIN_BATCH(4);
> -   OUT_BATCH(_3DSTATE_CLIP << 16 | (4 - 2));
> -   OUT_BATCH(dw1);
> -   OUT_BATCH(enable |
> -	     GEN6_CLIP_MODE_NORMAL |
> -	     dw2);
> -   OUT_BATCH(U_FIXED(0.125, 3) << GEN6_CLIP_MIN_POINT_WIDTH_SHIFT |
> -             U_FIXED(255.875, 3) << GEN6_CLIP_MAX_POINT_WIDTH_SHIFT |
> -             (_mesa_geometric_layers(fb) > 0 ? 0 : GEN6_CLIP_FORCE_ZERO_RTAINDEX) |
> -             ((viewport_count - 1) & GEN6_CLIP_MAX_VP_INDEX_MASK));
> -   ADVANCE_BATCH();
> -}
> -
> -const struct brw_tracked_state gen6_clip_state = {
> -   .dirty = {
> -      .mesa  = _NEW_BUFFERS |
> -               _NEW_LIGHT |
> -               _NEW_POLYGON |
> -               _NEW_TRANSFORM,
> -      .brw   = BRW_NEW_BLORP |
> -               BRW_NEW_CONTEXT |
> -               BRW_NEW_FS_PROG_DATA |
> -               BRW_NEW_GS_PROG_DATA |
> -               BRW_NEW_VS_PROG_DATA |
> -               BRW_NEW_META_IN_PROGRESS |
> -               BRW_NEW_PRIMITIVE |
> -               BRW_NEW_RASTERIZER_DISCARD |
> -               BRW_NEW_TES_PROG_DATA |
> -               BRW_NEW_VIEWPORT_COUNT,
> -   },
> -   .emit = upload_clip_state,
> -};
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index ff28cf5..7532085 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -32,6 +32,8 @@
>  #include "intel_batchbuffer.h"
>  #include "intel_fbo.h"
>  
> +#include "main/fbobject.h"
> +#include "main/framebuffer.h"
>  #include "main/stencil.h"
>  
>  UNUSED static void *
> @@ -208,6 +210,141 @@ static const struct brw_tracked_state genX(depth_stencil_state) = {
>  
>  /* ---------------------------------------------------------------------- */
>  
> +static void
> +genX(upload_clip_state)(struct brw_context *brw)
> +{
> +   struct gl_context *ctx = &brw->ctx;
> +
> +   /* _NEW_BUFFERS */
> +   struct gl_framebuffer *fb = ctx->DrawBuffer;

Let's add:

   /* BRW_NEW_WM_PROG_DATA */
   struct brw_wm_prog_data *wm_prog_data =
      brw_wm_prog_data(brw->wm.base.prog_data);

> +
> +   brw_batch_emit(brw, GENX(3DSTATE_CLIP), clip) {
> +      if (!brw->meta_in_progress)
> +         clip.StatisticsEnable = true;

How about:

      clip.StatisticsEnable = !brw->meta_in_progress;

> +
> +      /* TODO: How to properly check for this?
> +       * For now, copying code form vulkan
> +       */
> +      if (brw_wm_prog_data(brw->wm.base.prog_data)->barycentric_interp_modes &

With the new pointer, we can do:

   if (wm_prog_data->barycentric_interp_modes & 0x38)
      clip.NonPerspectiveBarycentricEnable = true;

(or BRW_BARYCENTRIC_NONPERSPECTIVE_BITS instead of 0x38)

> +          0x38)
> +         clip.NonPerspectiveBarycentricEnable = true;
> +
> +      clip.UserClipDistanceCullTestEnableBitmask =
> +         brw_vue_prog_data(brw->vs.base.prog_data)->cull_distance_mask;

3DSTATE_CLIP's CullTestEnableBitmask is unused on Gen8+, so we can put
this in a GEN_GEN < 8 check.  (We set it unconditionally in the old code
because I thought it was silly to add an extra runtime check to avoid
setting a couple harmless bits...but now that we can compile-time
check...we should.)  Perhaps move it below to the GEN_GEN < 8 block.

> +
> +#if GEN_GEN >= 7
> +      clip.EarlyCullEnable = true;
> +#endif
> +
> +#if GEN_GEN == 7
> +      if (ctx->Polygon._FrontBit == _mesa_is_user_fbo(fb))
> +         clip.FrontWinding = 1;

Please use true instead of 1, or eliminate the if and do:

      clip.FrontWinding = ctx->Polygon._FrontBit == _mesa_is_user_fbo(fb);

> +
> +      if (ctx->Polygon.CullFlag) {
> +         switch (ctx->Polygon.CullFaceMode) {
> +            case GL_FRONT:

Mesa style is to align 'case' with 'switch'.

> +               clip.CullMode = CULLMODE_FRONT;
> +               break;
> +            case GL_BACK:
> +               clip.CullMode = CULLMODE_BACK;
> +               break;
> +            case GL_FRONT_AND_BACK:
> +               clip.CullMode = CULLMODE_BOTH;
> +               break;
> +            default:
> +               unreachable("Should not get here: invalid CullFlag");
> +         }
> +      } else {
> +         clip.CullMode = CULLMODE_NONE;
> +      }
> +#endif
> +
> +#if GEN_GEN < 8
> +      if (!ctx->Transform.DepthClamp)
> +         clip.ViewportZClipTestEnable = true;

      clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp;

> +#endif
> +
> +      /* _NEW_LIGHT */
> +      if (ctx->Light.ProvokingVertex == GL_FIRST_VERTEX_CONVENTION) {
> +         clip.TriangleStripListProvokingVertexSelect = 0;
> +         clip.TriangleFanProvokingVertexSelect = 1;
> +         clip.LineStripListProvokingVertexSelect = 0;
> +      } else {
> +         clip.TriangleStripListProvokingVertexSelect = 2;
> +         clip.TriangleFanProvokingVertexSelect = 2;
> +         clip.LineStripListProvokingVertexSelect = 1;
> +      }
> +
> +      /* _NEW_TRANSFORM */
> +      clip.UserClipDistanceClipTestEnableBitmask =
> +         ctx->Transform.ClipPlanesEnabled;
> +
> +#if GEN_GEN >= 8
> +      clip.ForceUserClipDistanceClipTestEnableBitmask = true;
> +#endif
> +
> +      if (ctx->Transform.ClipDepthMode == GL_ZERO_TO_ONE)
> +         clip.APIMode = APIMODE_D3D;
> +      else
> +         clip.APIMode = APIMODE_OGL;
> +
> +      clip.GuardbandClipTestEnable = true;
> +
> +      /* BRW_NEW_VIEWPORT_COUNT */
> +      const unsigned viewport_count = brw->clip.viewport_count;
> +
> +      if (ctx->RasterDiscard) {
> +         clip.ClipMode = CLIPMODE_REJECT_ALL;
> +#if GEN_GEN == 6
> +         perf_debug("Rasterizer discard is currently implemented via the "
> +                    "clipper; having the GS not write primitives would "
> +                    "likely be faster.\n");
> +#endif
> +      } else {
> +         clip.ClipMode = CLIPMODE_NORMAL;
> +      }
> +
> +      if (brw->primitive == _3DPRIM_RECTLIST)
> +         clip.ClipEnable = false;
> +      else
> +         clip.ClipEnable = true;

      clip.ClipEnable = brw->primitive != _3DPRIM_RECTLIST;

> +
> +      /* _NEW_POLYGON,
> +       * BRW_NEW_GEOMETRY_PROGRAM | BRW_NEW_TES_PROG_DATA | BRW_NEW_PRIMITIVE
> +       */
> +      if (!brw_is_drawing_points(brw) && !brw_is_drawing_lines(brw))
> +         clip.ViewportXYClipTestEnable = true;
> +
> +      clip.MinimumPointWidth = 0.125;
> +      clip.MaximumPointWidth = 255.875;
> +      clip.MaximumVPIndex = viewport_count - 1;

> +      if (_mesa_geometric_layers(fb) <= 0)

This is an unsigned value, please make this == 0.  (It makes more sense
that way...if zero, force zero...)

> +         clip.ForceZeroRTAIndexEnable = true;
> +   }
> +}
> +
> +static const struct brw_tracked_state genX(clip_state) = {
> +   .dirty = {
> +      .mesa  = _NEW_BUFFERS |
> +               _NEW_LIGHT |
> +               _NEW_POLYGON |
> +               _NEW_TRANSFORM,
> +      .brw   = BRW_NEW_BLORP |
> +               BRW_NEW_CONTEXT |
> +               BRW_NEW_FS_PROG_DATA |
> +               BRW_NEW_GS_PROG_DATA |
> +               BRW_NEW_VS_PROG_DATA |
> +               BRW_NEW_META_IN_PROGRESS |
> +               BRW_NEW_PRIMITIVE |
> +               BRW_NEW_RASTERIZER_DISCARD |
> +               BRW_NEW_TES_PROG_DATA |
> +               BRW_NEW_VIEWPORT_COUNT,
> +   },
> +   .emit = genX(upload_clip_state),
> +};
> +
> +/* ---------------------------------------------------------------------- */
> +
>  #endif
>  
>  void
> @@ -315,7 +452,7 @@ genX(init_atoms)(struct brw_context *brw)
>  
>        &gen6_vs_state,
>        &gen6_gs_state,
> -      &gen6_clip_state,
> +      &genX(clip_state),
>        &gen6_sf_state,
>        &gen6_wm_state,
>  
> @@ -403,7 +540,7 @@ genX(init_atoms)(struct brw_context *brw)
>        &gen7_ds_state,
>        &gen7_gs_state,
>        &gen7_sol_state,
> -      &gen6_clip_state,
> +      &genX(clip_state),
>        &gen7_sbe_state,
>        &gen7_sf_state,
>        &gen7_wm_state,
> @@ -490,7 +627,7 @@ genX(init_atoms)(struct brw_context *brw)
>        &gen8_ds_state,
>        &gen8_gs_state,
>        &gen7_sol_state,
> -      &gen6_clip_state,
> +      &genX(clip_state),
>        &gen8_raster_state,
>        &gen8_sbe_state,
>        &gen8_sf_state,
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170425/82a214a1/attachment-0001.sig>


More information about the mesa-dev mailing list