Re: [Mesa-dev] [Mesa-stable] [PATCH] i965: Don't emit SURFACE_STATEs for gather workarounds on Broadwell.

2014-06-23 Thread Kenneth Graunke
It did not.  But, I just ran all of those tests with and without this patch, 
and there were no regressions.  So, I think we're good.

Pushed - thanks for the review!

On Thursday, May 29, 2014 07:32:21 PM Chris Forbes wrote:
> Did this test run include forcing ARB_gpu_shader5 on?
> 
> On Thu, May 29, 2014 at 7:06 PM, Kenneth Graunke  
wrote:
> > As far as I can tell, Broadwell doesn't need any of the SURFACE_STATE
> > workarounds for textureGather() bugs, so there's no need to emit
> > a second set of identical copies.
> >
> > To keep things simple, just point the gather surface index base to the
> > same place as the texture surface index base.
> >
> > Signed-off-by: Kenneth Graunke 
> > Cc: "10.2" 
> > ---
> >  src/mesa/drivers/dri/i965/brw_shader.cpp |  9 +++--
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 14 --
> >  2 files changed, 15 insertions(+), 8 deletions(-)
> >
> > No Piglit regressions on Broadwell with -t texture.
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > index f4f1334..7fae416 100644
> > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > @@ -740,8 +740,13 @@ 
backend_visitor::assign_common_binding_table_offsets(uint32_t 
next_binding_table
> > }
> >
> > if (prog->UsesGather) {
> > -  stage_prog_data->binding_table.gather_texture_start = 
next_binding_table_offset;
> > -  next_binding_table_offset += num_textures;
> > +  if (brw->gen >= 8) {
> > + stage_prog_data->binding_table.gather_texture_start =
> > +stage_prog_data->binding_table.texture_start;
> > +  } else {
> > + stage_prog_data->binding_table.gather_texture_start = 
next_binding_table_offset;
> > + next_binding_table_offset += num_textures;
> > +  }
> > } else {
> >stage_prog_data->binding_table.gather_texture_start = 0xd0d0d0d0;
> > }
> > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > index c9d9614..3279d3b 100644
> > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > @@ -829,12 +829,14 @@ brw_update_texture_surfaces(struct brw_context *brw)
> > /* emit alternate set of surface state for gather. this
> >  * allows the surface format to be overriden for only the
> >  * gather4 messages. */
> > -   if (vs && vs->UsesGather)
> > -  update_stage_texture_surfaces(brw, vs, &brw->vs.base, true);
> > -   if (gs && gs->UsesGather)
> > -  update_stage_texture_surfaces(brw, gs, &brw->gs.base, true);
> > -   if (fs && fs->UsesGather)
> > -  update_stage_texture_surfaces(brw, fs, &brw->wm.base, true);
> > +   if (brw->gen < 8) {
> > +  if (vs && vs->UsesGather)
> > + update_stage_texture_surfaces(brw, vs, &brw->vs.base, true);
> > +  if (gs && gs->UsesGather)
> > + update_stage_texture_surfaces(brw, gs, &brw->gs.base, true);
> > +  if (fs && fs->UsesGather)
> > + update_stage_texture_surfaces(brw, fs, &brw->wm.base, true);
> > +   }
> >
> > brw->state.dirty.brw |= BRW_NEW_SURFACES;
> >  }
> > --
> > 1.9.1
> >
> > ___
> > mesa-stable mailing list
> > mesa-sta...@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-stable


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [Mesa-stable] [PATCH] i965: Don't emit SURFACE_STATEs for gather workarounds on Broadwell.

2014-05-29 Thread Chris Forbes
Reviewed-by: Chris Forbes 

On Thu, May 29, 2014 at 7:32 PM, Chris Forbes  wrote:
> Did this test run include forcing ARB_gpu_shader5 on?
>
> On Thu, May 29, 2014 at 7:06 PM, Kenneth Graunke  
> wrote:
>> As far as I can tell, Broadwell doesn't need any of the SURFACE_STATE
>> workarounds for textureGather() bugs, so there's no need to emit
>> a second set of identical copies.
>>
>> To keep things simple, just point the gather surface index base to the
>> same place as the texture surface index base.
>>
>> Signed-off-by: Kenneth Graunke 
>> Cc: "10.2" 
>> ---
>>  src/mesa/drivers/dri/i965/brw_shader.cpp |  9 +++--
>>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 14 --
>>  2 files changed, 15 insertions(+), 8 deletions(-)
>>
>> No Piglit regressions on Broadwell with -t texture.
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
>> b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> index f4f1334..7fae416 100644
>> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> @@ -740,8 +740,13 @@ 
>> backend_visitor::assign_common_binding_table_offsets(uint32_t 
>> next_binding_table
>> }
>>
>> if (prog->UsesGather) {
>> -  stage_prog_data->binding_table.gather_texture_start = 
>> next_binding_table_offset;
>> -  next_binding_table_offset += num_textures;
>> +  if (brw->gen >= 8) {
>> + stage_prog_data->binding_table.gather_texture_start =
>> +stage_prog_data->binding_table.texture_start;
>> +  } else {
>> + stage_prog_data->binding_table.gather_texture_start = 
>> next_binding_table_offset;
>> + next_binding_table_offset += num_textures;
>> +  }
>> } else {
>>stage_prog_data->binding_table.gather_texture_start = 0xd0d0d0d0;
>> }
>> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
>> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> index c9d9614..3279d3b 100644
>> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> @@ -829,12 +829,14 @@ brw_update_texture_surfaces(struct brw_context *brw)
>> /* emit alternate set of surface state for gather. this
>>  * allows the surface format to be overriden for only the
>>  * gather4 messages. */
>> -   if (vs && vs->UsesGather)
>> -  update_stage_texture_surfaces(brw, vs, &brw->vs.base, true);
>> -   if (gs && gs->UsesGather)
>> -  update_stage_texture_surfaces(brw, gs, &brw->gs.base, true);
>> -   if (fs && fs->UsesGather)
>> -  update_stage_texture_surfaces(brw, fs, &brw->wm.base, true);
>> +   if (brw->gen < 8) {
>> +  if (vs && vs->UsesGather)
>> + update_stage_texture_surfaces(brw, vs, &brw->vs.base, true);
>> +  if (gs && gs->UsesGather)
>> + update_stage_texture_surfaces(brw, gs, &brw->gs.base, true);
>> +  if (fs && fs->UsesGather)
>> + update_stage_texture_surfaces(brw, fs, &brw->wm.base, true);
>> +   }
>>
>> brw->state.dirty.brw |= BRW_NEW_SURFACES;
>>  }
>> --
>> 1.9.1
>>
>> ___
>> mesa-stable mailing list
>> mesa-sta...@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [Mesa-stable] [PATCH] i965: Don't emit SURFACE_STATEs for gather workarounds on Broadwell.

2014-05-29 Thread Chris Forbes
Did this test run include forcing ARB_gpu_shader5 on?

On Thu, May 29, 2014 at 7:06 PM, Kenneth Graunke  wrote:
> As far as I can tell, Broadwell doesn't need any of the SURFACE_STATE
> workarounds for textureGather() bugs, so there's no need to emit
> a second set of identical copies.
>
> To keep things simple, just point the gather surface index base to the
> same place as the texture surface index base.
>
> Signed-off-by: Kenneth Graunke 
> Cc: "10.2" 
> ---
>  src/mesa/drivers/dri/i965/brw_shader.cpp |  9 +++--
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 14 --
>  2 files changed, 15 insertions(+), 8 deletions(-)
>
> No Piglit regressions on Broadwell with -t texture.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
> b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index f4f1334..7fae416 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -740,8 +740,13 @@ 
> backend_visitor::assign_common_binding_table_offsets(uint32_t 
> next_binding_table
> }
>
> if (prog->UsesGather) {
> -  stage_prog_data->binding_table.gather_texture_start = 
> next_binding_table_offset;
> -  next_binding_table_offset += num_textures;
> +  if (brw->gen >= 8) {
> + stage_prog_data->binding_table.gather_texture_start =
> +stage_prog_data->binding_table.texture_start;
> +  } else {
> + stage_prog_data->binding_table.gather_texture_start = 
> next_binding_table_offset;
> + next_binding_table_offset += num_textures;
> +  }
> } else {
>stage_prog_data->binding_table.gather_texture_start = 0xd0d0d0d0;
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index c9d9614..3279d3b 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -829,12 +829,14 @@ brw_update_texture_surfaces(struct brw_context *brw)
> /* emit alternate set of surface state for gather. this
>  * allows the surface format to be overriden for only the
>  * gather4 messages. */
> -   if (vs && vs->UsesGather)
> -  update_stage_texture_surfaces(brw, vs, &brw->vs.base, true);
> -   if (gs && gs->UsesGather)
> -  update_stage_texture_surfaces(brw, gs, &brw->gs.base, true);
> -   if (fs && fs->UsesGather)
> -  update_stage_texture_surfaces(brw, fs, &brw->wm.base, true);
> +   if (brw->gen < 8) {
> +  if (vs && vs->UsesGather)
> + update_stage_texture_surfaces(brw, vs, &brw->vs.base, true);
> +  if (gs && gs->UsesGather)
> + update_stage_texture_surfaces(brw, gs, &brw->gs.base, true);
> +  if (fs && fs->UsesGather)
> + update_stage_texture_surfaces(brw, fs, &brw->wm.base, true);
> +   }
>
> brw->state.dirty.brw |= BRW_NEW_SURFACES;
>  }
> --
> 1.9.1
>
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev