[Mesa-dev] [PATCH 1/2] i965: Disable point coords when not rendering points.

[Kenneth Graunke]

On 02/16/2013 07:29 AM, Paul Berry wrote:
>  From the Ivy Bridge PRM, p268 (3DSTATE_SBE), in the description of
> "Point Sprite Texture Coordinate Enable":
>
>      "This field must be programmed to 0 when non-point primitives are
>      rendered."
>
> A similar admonishment exists in the Sandy Bridge PRM.
>
> Although I'm not aware of any known bugs due to us ignoring this,
> defying the hardware docs is generally unwise.
> ---
>   src/mesa/drivers/dri/i965/gen6_sf_state.c | 9 ++++++---
>   src/mesa/drivers/dri/i965/gen7_sf_state.c | 9 ++++++---
>   2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> index 7071acc..d88c49a 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> @@ -127,6 +127,8 @@ upload_sf_state(struct brw_context *brw)
>      /* _NEW_BUFFER */
>      bool render_to_fbo = _mesa_is_user_fbo(brw->intel.ctx.DrawBuffer);
>      bool multisampled_fbo = ctx->DrawBuffer->Visual.samples > 1;
> +   /* BRW_NEW_REDUCED_PRIMITIVE */
> +   bool is_point_primitive = intel->reduced_primitive == GL_POINTS;

NAK.  intel->reduced_primitive is only set in brw_set_prim(), which is 
never called on Gen6+.  (gen6_set_prim() gets called instead.)

I believe you want brw->primitive and BRW_NEW_PRIMITIVE instead.

>      int attr = 0, input_index = 0;
>      int urb_entry_read_offset = 1;
> @@ -280,13 +282,13 @@ upload_sf_state(struct brw_context *brw)
>   	 continue;
>
>         /* _NEW_POINT */
> -      if (ctx->Point.PointSprite &&
> +      if (is_point_primitive && ctx->Point.PointSprite &&
>   	  (attr >= FRAG_ATTRIB_TEX0 && attr <= FRAG_ATTRIB_TEX7) &&
>   	  ctx->Point.CoordReplace[attr - FRAG_ATTRIB_TEX0]) {
>   	 dw16 |= (1 << input_index);
>         }
>
> -      if (attr == FRAG_ATTRIB_PNTC)
> +      if (is_point_primitive && attr == FRAG_ATTRIB_PNTC)
>   	 dw16 |= (1 << input_index);
>
>         /* flat shading */
> @@ -360,7 +362,8 @@ const struct brw_tracked_state gen6_sf_state = {
>   		_NEW_POINT |
>                   _NEW_MULTISAMPLE),
>         .brw   = (BRW_NEW_CONTEXT |
> -		BRW_NEW_FRAGMENT_PROGRAM),
> +		BRW_NEW_FRAGMENT_PROGRAM |
> +                BRW_NEW_REDUCED_PRIMITIVE),
>         .cache = CACHE_NEW_VS_PROG
>      },
>      .emit = upload_sf_state,
> diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> index 9171eff..0f6252b 100644
> --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> @@ -46,6 +46,8 @@ upload_sbe_state(struct brw_context *brw)
>      /* _NEW_BUFFERS */
>      bool render_to_fbo = _mesa_is_user_fbo(ctx->DrawBuffer);
>      uint32_t point_sprite_origin;
> +   /* BRW_NEW_REDUCED_PRIMITIVE */
> +   bool is_point_primitive = intel->reduced_primitive == GL_POINTS;
>
>      /* FINISHME: Attribute Swizzle Control Mode? */
>      dw1 = GEN7_SBE_SWIZZLE_ENABLE | num_outputs << GEN7_SBE_NUM_OUTPUTS_SHIFT;
> @@ -78,13 +80,13 @@ upload_sbe_state(struct brw_context *brw)
>         if (!(brw->fragment_program->Base.InputsRead & BITFIELD64_BIT(attr)))
>   	 continue;
>
> -      if (ctx->Point.PointSprite &&
> +      if (is_point_primitive && ctx->Point.PointSprite &&
>   	  attr >= FRAG_ATTRIB_TEX0 && attr <= FRAG_ATTRIB_TEX7 &&
>   	  ctx->Point.CoordReplace[attr - FRAG_ATTRIB_TEX0]) {
>   	 dw10 |= (1 << input_index);
>         }
>
> -      if (attr == FRAG_ATTRIB_PNTC)
> +      if (is_point_primitive && attr == FRAG_ATTRIB_PNTC)
>   	 dw10 |= (1 << input_index);
>
>         /* flat shading */
> @@ -149,7 +151,8 @@ const struct brw_tracked_state gen7_sbe_state = {
>   		_NEW_POINT |
>   		_NEW_PROGRAM),
>         .brw   = (BRW_NEW_CONTEXT |
> -		BRW_NEW_FRAGMENT_PROGRAM),
> +		BRW_NEW_FRAGMENT_PROGRAM |
> +                BRW_NEW_REDUCED_PRIMITIVE),
>         .cache = CACHE_NEW_VS_PROG
>      },
>      .emit = upload_sbe_state,
>