Re: [Mesa-dev] [PATCH 2/2] i965: Fix state flagging of Gen6 SOL programs.

[Jordan Justen]

On 2017-08-22 19:09:23, Kenneth Graunke wrote:
> It doesn't seem like the old code could possibly work.
> 
> 1. brw_gs_state_dirty made us bail unless one of these flags were set:
>_NEW_TEXTURE, BRW_NEW_GEOMETRY_PROGRAM, BRW_NEW_TRANSFORM_FEEDBACK
> 2. If there was no geometry program, we called brw_upload_ff_gs_prog()3

bonus '3'.

Series Reviewed-by: Jordan Justen 

> 3. That checked brw_ff_gs_state_dirty and bailed unless these were set:
>_NEW_LIGHT, BRW_NEW_PRIMITIVE, BRW_NEW_TRANSFORM_FEEDBACK,
>BRW_NEW_VS_PROG_DATA.
> 4. brw_ff_gs_prog_key pv_first and attr fields were set based on data
>depending on _NEW_LIGHT and BRW_NEW_VS_PROG_DATA.
> 
> This means that if we needed a FF GS program, and changed the VS
> outputs or provoking vertex mode, we'd fail to notice that we needed
> to emit a new program.
> ---
>  src/mesa/drivers/dri/i965/brw_gs.c   | 16 
>  src/mesa/drivers/dri/i965/brw_state_upload.c |  9 ++---
>  2 files changed, 6 insertions(+), 19 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c 
> b/src/mesa/drivers/dri/i965/brw_gs.c
> index 9ba94c45c8f..179ccc4c6fb 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> @@ -206,22 +206,6 @@ brw_upload_gs_prog(struct brw_context *brw)
> if (!brw_gs_state_dirty(brw))
>return;
>  
> -   if (gp == NULL) {
> -  /* No geometry shader.  Vertex data just passes straight through. */
> -  if (brw->gen == 6 &&
> -  (brw->ctx.NewDriverState & BRW_NEW_TRANSFORM_FEEDBACK)) {
> - brw_upload_ff_gs_prog(brw);
> - return;
> -  }
> -
> -  /* Other state atoms had better not try to access prog_data, since
> -   * there's no GS program.
> -   */
> -  brw->gs.base.prog_data = NULL;
> -
> -  return;
> -   }
> -
> brw_gs_populate_key(brw, );
>  
> if (!brw_search_cache(>cache, BRW_CACHE_GS_PROG,
> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
> b/src/mesa/drivers/dri/i965/brw_state_upload.c
> index 1ae45ba2ac1..1608af4f3bd 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> @@ -395,10 +395,13 @@ brw_upload_programs(struct brw_context *brw,
>brw_upload_vs_prog(brw);
>brw_upload_tess_programs(brw);
>  
> -  if (brw->gen < 6)
> - brw_upload_ff_gs_prog(brw);
> -  else
> +  if (brw->geometry_program) {
>   brw_upload_gs_prog(brw);
> +  } else {
> + brw->gs.base.prog_data = NULL;
> + if (brw->gen < 7)
> +brw_upload_ff_gs_prog(brw);
> +  }
>  
>/* Update the VUE map for data exiting the GS stage of the pipeline.
> * This comes from the last enabled shader stage.
> -- 
> 2.14.1
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] i965: Fix state flagging of Gen6 SOL programs.

[Kenneth Graunke]

It doesn't seem like the old code could possibly work.

1. brw_gs_state_dirty made us bail unless one of these flags were set:
   _NEW_TEXTURE, BRW_NEW_GEOMETRY_PROGRAM, BRW_NEW_TRANSFORM_FEEDBACK
2. If there was no geometry program, we called brw_upload_ff_gs_prog()3
3. That checked brw_ff_gs_state_dirty and bailed unless these were set:
   _NEW_LIGHT, BRW_NEW_PRIMITIVE, BRW_NEW_TRANSFORM_FEEDBACK,
   BRW_NEW_VS_PROG_DATA.
4. brw_ff_gs_prog_key pv_first and attr fields were set based on data
   depending on _NEW_LIGHT and BRW_NEW_VS_PROG_DATA.

This means that if we needed a FF GS program, and changed the VS
outputs or provoking vertex mode, we'd fail to notice that we needed
to emit a new program.
---
 src/mesa/drivers/dri/i965/brw_gs.c   | 16 
 src/mesa/drivers/dri/i965/brw_state_upload.c |  9 ++---
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_gs.c 
b/src/mesa/drivers/dri/i965/brw_gs.c
index 9ba94c45c8f..179ccc4c6fb 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_gs.c
@@ -206,22 +206,6 @@ brw_upload_gs_prog(struct brw_context *brw)
if (!brw_gs_state_dirty(brw))
   return;
 
-   if (gp == NULL) {
-  /* No geometry shader.  Vertex data just passes straight through. */
-  if (brw->gen == 6 &&
-  (brw->ctx.NewDriverState & BRW_NEW_TRANSFORM_FEEDBACK)) {
- brw_upload_ff_gs_prog(brw);
- return;
-  }
-
-  /* Other state atoms had better not try to access prog_data, since
-   * there's no GS program.
-   */
-  brw->gs.base.prog_data = NULL;
-
-  return;
-   }
-
brw_gs_populate_key(brw, );
 
if (!brw_search_cache(>cache, BRW_CACHE_GS_PROG,
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 1ae45ba2ac1..1608af4f3bd 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -395,10 +395,13 @@ brw_upload_programs(struct brw_context *brw,
   brw_upload_vs_prog(brw);
   brw_upload_tess_programs(brw);
 
-  if (brw->gen < 6)
- brw_upload_ff_gs_prog(brw);
-  else
+  if (brw->geometry_program) {
  brw_upload_gs_prog(brw);
+  } else {
+ brw->gs.base.prog_data = NULL;
+ if (brw->gen < 7)
+brw_upload_ff_gs_prog(brw);
+  }
 
   /* Update the VUE map for data exiting the GS stage of the pipeline.
* This comes from the last enabled shader stage.
-- 
2.14.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev