[Mesa-dev] [RFC PATCH 6/6] i965: add EXT_polygon_offset_clamp support to gen4/gen5

Ilia Mirkin imirkin at alum.mit.edu
Sun Feb 1 09:07:32 PST 2015


On Sun, Feb 1, 2015 at 10:18 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>
> Thoroughly untested, beyond compilation. Don't have the HW, nor am I
> particularly familiar with the i965 driver in general. Should be easy enough
> to test though for someone with the hw.
>
>  src/mesa/drivers/dri/i965/brw_clip.c          |  1 +
>  src/mesa/drivers/dri/i965/brw_clip.h          |  1 +
>  src/mesa/drivers/dri/i965/brw_clip_unfilled.c | 14 ++++++++++++++
>  src/mesa/drivers/dri/i965/brw_context.h       |  2 ++
>  src/mesa/drivers/dri/i965/brw_misc_state.c    |  8 --------
>  src/mesa/drivers/dri/i965/brw_wm_state.c      | 11 +++++++++++
>  src/mesa/drivers/dri/i965/intel_extensions.c  |  2 +-
>  7 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.c b/src/mesa/drivers/dri/i965/brw_clip.c
> index 3fef38c..a5056fd 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip.c
> @@ -222,6 +222,7 @@ brw_upload_clip_prog(struct brw_context *brw)
>                /* _NEW_POLYGON, _NEW_BUFFERS */
>                key.offset_units = ctx->Polygon.OffsetUnits * ctx->DrawBuffer->_MRD * 2;
>                key.offset_factor = ctx->Polygon.OffsetFactor * ctx->DrawBuffer->_MRD;
> +              key.offset_clamp = ctx->Polygon.OffsetClamp * ctx->DrawBuffer->_MRD;
>             }
>
>             switch (ctx->Polygon.FrontFace) {
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.h b/src/mesa/drivers/dri/i965/brw_clip.h
> index d085daf..8a68f10 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.h
> +++ b/src/mesa/drivers/dri/i965/brw_clip.h
> @@ -62,6 +62,7 @@ struct brw_clip_prog_key {
>
>     GLfloat offset_factor;
>     GLfloat offset_units;
> +   GLfloat offset_clamp;
>  };
>
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_unfilled.c b/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
> index 82d7b64..09c1e1d 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
> @@ -188,6 +188,12 @@ static void copy_bfc( struct brw_clip_compile *c )
>    GLfloat bc   = dir.y * iz;
>    offset = ctx->Polygon.OffsetUnits * DEPTH_SCALE;
>    offset += MAX2( abs(ac), abs(bc) ) * ctx->Polygon.OffsetFactor;
> +  if (ctx->Polygon.OffsetClamp && isfinite(ctx->Polygon.OffsetClamp)) {
> +    if (ctx->Polygon.OffsetClamp < 0)
> +      offset = MAX2( offset, ctx->Polygon.OffsetClamp );
> +    else
> +      offset = MIN2( offset, ctx->Polygon.OffsetClamp );
> +  }
>    offset *= MRD;
>  */
>  static void compute_offset( struct brw_clip_compile *c )
> @@ -212,6 +218,14 @@ static void compute_offset( struct brw_clip_compile *c )
>
>     brw_MUL(p, vec1(off), vec1(off), brw_imm_f(c->key.offset_factor));
>     brw_ADD(p, vec1(off), vec1(off), brw_imm_f(c->key.offset_units));
> +   if (c->key.offset_clamp && isfinite(c->key.offset_clamp)) {
> +      brw_CMP(p,
> +              vec1(brw_null_reg()),
> +              c->key.offset_clamp < 0 ? BRW_CONDITIONAL_G : BRW_CONDITIONAL_L,
> +              vec1(off),
> +              brw_imm_f(c->key.offset_clamp));
> +      brw_SEL(p, vec1(off), vec1(off), brw_imm_f(c->key.offset_clamp));
> +   }

A question for whoever reviews -- is this right? I'm (a) confused
about how CMP/SEL work, (b) unsure if it's OK to stick the immediates
into the various instructions, since I assume this happens outside of
some normalize-everything pass, and (c) I could definitely have gotten
the _G vs _L thing backwards, please verify against the pseudocode
above (which I'm fairly certain is right).

>  }
>
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index 6195d3d..6041824 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1329,6 +1329,8 @@ struct brw_context
>         */
>        drm_intel_bo *multisampled_null_render_target_bo;
>        uint32_t fast_clear_op;
> +
> +      float offset_clamp;
>     } wm;
>
>
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
> index a405eb2..522bde8 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -894,14 +894,6 @@ brw_upload_invariant_state(struct brw_context *brw)
>     OUT_BATCH(_3DSTATE_PIPELINE_SELECT << 16 | (brw->gen >= 9 ? (3 << 8) : 0));
>     ADVANCE_BATCH();
>
> -   if (brw->gen < 6) {
> -      /* Disable depth offset clamping. */
> -      BEGIN_BATCH(2);
> -      OUT_BATCH(_3DSTATE_GLOBAL_DEPTH_OFFSET_CLAMP << 16 | (2 - 2));
> -      OUT_BATCH_F(0.0);
> -      ADVANCE_BATCH();
> -   }
> -
>     if (brw->gen >= 8) {
>        BEGIN_BATCH(3);
>        OUT_BATCH(CMD_STATE_SIP << 16 | (3 - 2));
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_state.c b/src/mesa/drivers/dri/i965/brw_wm_state.c
> index 0dee1f8..ed6c46d 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_state.c
> @@ -31,6 +31,7 @@
>
>
>
> +#include "intel_batchbuffer.h"
>  #include "intel_fbo.h"
>  #include "brw_context.h"
>  #include "brw_state.h"
> @@ -238,6 +239,16 @@ brw_upload_wm_unit(struct brw_context *brw)
>     }
>
>     brw->state.dirty.brw |= BRW_NEW_GEN4_UNIT_STATE;
> +
> +   /* _NEW_POLGYON */
> +   if (brw->wm.offset_clamp != ctx->Polygon.OffsetClamp) {
> +      BEGIN_BATCH(2);
> +      OUT_BATCH(_3DSTATE_GLOBAL_DEPTH_OFFSET_CLAMP << 16 | (2 - 2));
> +      OUT_BATCH_F(ctx->Polygon.OffsetClamp);
> +      ADVANCE_BATCH();
> +
> +      brw->wm.offset_clamp = ctx->Polygon.OffsetClamp;
> +   }

And is this the right place (and method) to do something like that?
sf/wm -- all the same to me :) It did seem to be grouped with the
other "wm" stuff in the docs though. I also didn't see any of those do
batches because these things are uploaded in a sneaky way that I
didn't fully understand, but this is how the clamping was being
disabled globally before... Lastly, do I need to emit a 0 there in
some state initialization routine? Wasn't sure if everything comes up
0'd out already.

>  }
>
>  const struct brw_tracked_state brw_wm_unit = {
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 4dacfd0..5e2f15b 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -229,6 +229,7 @@ intelInitExtensions(struct gl_context *ctx)
>     ctx->Extensions.EXT_packed_float = true;
>     ctx->Extensions.EXT_pixel_buffer_object = true;
>     ctx->Extensions.EXT_point_parameters = true;
> +   ctx->Extensions.EXT_polygon_offset_clamp = true;
>     ctx->Extensions.EXT_provoking_vertex = true;
>     ctx->Extensions.EXT_texture_array = true;
>     ctx->Extensions.EXT_texture_env_dot3 = true;
> @@ -285,7 +286,6 @@ intelInitExtensions(struct gl_context *ctx)
>        ctx->Extensions.ARB_texture_gather = true;
>        ctx->Extensions.ARB_conditional_render_inverted = true;
>        ctx->Extensions.AMD_vertex_shader_layer = true;
> -      ctx->Extensions.EXT_polygon_offset_clamp = true;
>
>        /* Test if the kernel has the ioctl. */
>        if (drm_intel_reg_read(brw->bufmgr, TIMESTAMP, &dummy) == 0)
> --
> 2.0.5
>


More information about the mesa-dev mailing list