Re: [Intel-gfx] [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state

2015-08-13 Thread Mika Kuoppala
Ben Widawsky  writes:

> On Fri, Jul 31, 2015 at 04:27:07PM +0100, Arun Siluvery wrote:
>> A programming restriction exists for this instruction, atleast one component
>> of one valid vertex element must be enabled.
>> 
>> Cc: Ben Widawsky 
>> Cc: Chris Wilson 
>> Signed-off-by: Arun Siluvery 
>> ---
>>  tools/null_state_gen/intel_renderstate_gen9.c | 7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tools/null_state_gen/intel_renderstate_gen9.c 
>> b/tools/null_state_gen/intel_renderstate_gen9.c
>> index 6f808f8..b3766ea 100644
>> --- a/tools/null_state_gen/intel_renderstate_gen9.c
>> +++ b/tools/null_state_gen/intel_renderstate_gen9.c
>> @@ -440,7 +440,12 @@ int gen9_setup_null_render_state(struct 
>> intel_batchbuffer *batch)
>>  /* Vertex buffers */
>>  gen8_emit_vertex_buffers(batch);
>>  gen8_emit_vertex_elements(batch);
>> -OUT_CMD(GEN9_3DSTATE_COMPONENT_PACKING, 5);
>> +
>> +OUT_BATCH(GEN9_3DSTATE_COMPONENT_PACKING | 3);
>> +OUT_BATCH(1);
>> +OUT_BATCH(0);
>> +OUT_BATCH(0);
>> +OUT_BATCH(0);
>>  
>>  OUT_BATCH(GEN6_3DSTATE_VF_STATISTICS | 1 /* Enable */);
>>  
>
> Like I said on IRC, I don't think this does anything unless you set bit 9 of
> 3DSTATE_VF.0. Also, I believe you should be setting 0xf, not 1 since you'd 
> want
> to use all 4 components.
>
> So I'm not really sure what we're aiming to do. If you make it 0xf, and add a
> comment about how this command doesn't do anything because we're not setting 
> up
> 3DSTATE_VF packing, it's:
> Reviewed-by: Ben Widawsky 
>

I messed with this one. The version with 0x1 went with your r-b.
The generator side change only so we can amend the version that
gets committed to kernel side still.

Sorry about this.
-Mika

> I'm still more in favor of dropping the command altogether, that too would be:
> Reviewed-by: Ben Widawsky 
>
> -- 
> Ben Widawsky, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state

2015-08-05 Thread Ben Widawsky
On Wed, Aug 05, 2015 at 11:13:46AM +0300, Mika Kuoppala wrote:
> Ben Widawsky  writes:
> 
> > On Fri, Jul 31, 2015 at 04:27:07PM +0100, Arun Siluvery wrote:
> >> A programming restriction exists for this instruction, atleast one 
> >> component
> >> of one valid vertex element must be enabled.
> >> 
> >> Cc: Ben Widawsky 
> >> Cc: Chris Wilson 
> >> Signed-off-by: Arun Siluvery 
> >> ---
> >>  tools/null_state_gen/intel_renderstate_gen9.c | 7 ++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/tools/null_state_gen/intel_renderstate_gen9.c 
> >> b/tools/null_state_gen/intel_renderstate_gen9.c
> >> index 6f808f8..b3766ea 100644
> >> --- a/tools/null_state_gen/intel_renderstate_gen9.c
> >> +++ b/tools/null_state_gen/intel_renderstate_gen9.c
> >> @@ -440,7 +440,12 @@ int gen9_setup_null_render_state(struct 
> >> intel_batchbuffer *batch)
> >>/* Vertex buffers */
> >>gen8_emit_vertex_buffers(batch);
> >>gen8_emit_vertex_elements(batch);
> >> -  OUT_CMD(GEN9_3DSTATE_COMPONENT_PACKING, 5);
> >> +
> >> +  OUT_BATCH(GEN9_3DSTATE_COMPONENT_PACKING | 3);
> >> +  OUT_BATCH(1);
> >> +  OUT_BATCH(0);
> >> +  OUT_BATCH(0);
> >> +  OUT_BATCH(0);
> >>  
> >>OUT_BATCH(GEN6_3DSTATE_VF_STATISTICS | 1 /* Enable */);
> >>  
> >
> > Like I said on IRC, I don't think this does anything unless you set bit 9 of
> > 3DSTATE_VF.0. Also, I believe you should be setting 0xf, not 1 since you'd 
> > want
> > to use all 4 components.
> >
> > So I'm not really sure what we're aiming to do. If you make it 0xf, and add 
> > a
> > comment about how this command doesn't do anything because we're not 
> > setting up
> > 3DSTATE_VF packing, it's:
> > Reviewed-by: Ben Widawsky 
> >
> > I'm still more in favor of dropping the command altogether, that too would 
> > be:
> > Reviewed-by: Ben Widawsky 
> 
> We are trying here just to satisfy the minimum requirements the bspec
> states.
> 

Right, except I am saying at least set minimal state which makes sense. Packing
1 component of the vertex element makes little sense (though doing this at all
makes even less sense).

> The bspec also states that with gen9+, the 3d pipeline state
> doesn't need to be initialized. We should seek confirmation
> about this and if it is so, remove the null state for gen9+.
> 

That would be super terrific.

> -Mika
> 
> >
> > -- 
> > Ben Widawsky, Intel Open Source Technology Center
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state

2015-08-05 Thread Mika Kuoppala
Ben Widawsky  writes:

> On Fri, Jul 31, 2015 at 04:27:07PM +0100, Arun Siluvery wrote:
>> A programming restriction exists for this instruction, atleast one component
>> of one valid vertex element must be enabled.
>> 
>> Cc: Ben Widawsky 
>> Cc: Chris Wilson 
>> Signed-off-by: Arun Siluvery 
>> ---
>>  tools/null_state_gen/intel_renderstate_gen9.c | 7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tools/null_state_gen/intel_renderstate_gen9.c 
>> b/tools/null_state_gen/intel_renderstate_gen9.c
>> index 6f808f8..b3766ea 100644
>> --- a/tools/null_state_gen/intel_renderstate_gen9.c
>> +++ b/tools/null_state_gen/intel_renderstate_gen9.c
>> @@ -440,7 +440,12 @@ int gen9_setup_null_render_state(struct 
>> intel_batchbuffer *batch)
>>  /* Vertex buffers */
>>  gen8_emit_vertex_buffers(batch);
>>  gen8_emit_vertex_elements(batch);
>> -OUT_CMD(GEN9_3DSTATE_COMPONENT_PACKING, 5);
>> +
>> +OUT_BATCH(GEN9_3DSTATE_COMPONENT_PACKING | 3);
>> +OUT_BATCH(1);
>> +OUT_BATCH(0);
>> +OUT_BATCH(0);
>> +OUT_BATCH(0);
>>  
>>  OUT_BATCH(GEN6_3DSTATE_VF_STATISTICS | 1 /* Enable */);
>>  
>
> Like I said on IRC, I don't think this does anything unless you set bit 9 of
> 3DSTATE_VF.0. Also, I believe you should be setting 0xf, not 1 since you'd 
> want
> to use all 4 components.
>
> So I'm not really sure what we're aiming to do. If you make it 0xf, and add a
> comment about how this command doesn't do anything because we're not setting 
> up
> 3DSTATE_VF packing, it's:
> Reviewed-by: Ben Widawsky 
>
> I'm still more in favor of dropping the command altogether, that too would be:
> Reviewed-by: Ben Widawsky 

We are trying here just to satisfy the minimum requirements the bspec
states.

The bspec also states that with gen9+, the 3d pipeline state
doesn't need to be initialized. We should seek confirmation
about this and if it is so, remove the null state for gen9+.

-Mika

>
> -- 
> Ben Widawsky, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state

2015-08-04 Thread Ben Widawsky
On Fri, Jul 31, 2015 at 04:27:07PM +0100, Arun Siluvery wrote:
> A programming restriction exists for this instruction, atleast one component
> of one valid vertex element must be enabled.
> 
> Cc: Ben Widawsky 
> Cc: Chris Wilson 
> Signed-off-by: Arun Siluvery 
> ---
>  tools/null_state_gen/intel_renderstate_gen9.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/null_state_gen/intel_renderstate_gen9.c 
> b/tools/null_state_gen/intel_renderstate_gen9.c
> index 6f808f8..b3766ea 100644
> --- a/tools/null_state_gen/intel_renderstate_gen9.c
> +++ b/tools/null_state_gen/intel_renderstate_gen9.c
> @@ -440,7 +440,12 @@ int gen9_setup_null_render_state(struct 
> intel_batchbuffer *batch)
>   /* Vertex buffers */
>   gen8_emit_vertex_buffers(batch);
>   gen8_emit_vertex_elements(batch);
> - OUT_CMD(GEN9_3DSTATE_COMPONENT_PACKING, 5);
> +
> + OUT_BATCH(GEN9_3DSTATE_COMPONENT_PACKING | 3);
> + OUT_BATCH(1);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
>  
>   OUT_BATCH(GEN6_3DSTATE_VF_STATISTICS | 1 /* Enable */);
>  

Like I said on IRC, I don't think this does anything unless you set bit 9 of
3DSTATE_VF.0. Also, I believe you should be setting 0xf, not 1 since you'd want
to use all 4 components.

So I'm not really sure what we're aiming to do. If you make it 0xf, and add a
comment about how this command doesn't do anything because we're not setting up
3DSTATE_VF packing, it's:
Reviewed-by: Ben Widawsky 

I'm still more in favor of dropping the command altogether, that too would be:
Reviewed-by: Ben Widawsky 

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state

2015-08-04 Thread Mika Kuoppala
Arun Siluvery  writes:

> A programming restriction exists for this instruction, atleast one component
> of one valid vertex element must be enabled.
>
> Cc: Ben Widawsky 
> Cc: Chris Wilson 
> Signed-off-by: Arun Siluvery 

Reviewed-by: Mika Kuoppala 

> ---
>  tools/null_state_gen/intel_renderstate_gen9.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tools/null_state_gen/intel_renderstate_gen9.c 
> b/tools/null_state_gen/intel_renderstate_gen9.c
> index 6f808f8..b3766ea 100644
> --- a/tools/null_state_gen/intel_renderstate_gen9.c
> +++ b/tools/null_state_gen/intel_renderstate_gen9.c
> @@ -440,7 +440,12 @@ int gen9_setup_null_render_state(struct 
> intel_batchbuffer *batch)
>   /* Vertex buffers */
>   gen8_emit_vertex_buffers(batch);
>   gen8_emit_vertex_elements(batch);
> - OUT_CMD(GEN9_3DSTATE_COMPONENT_PACKING, 5);
> +
> + OUT_BATCH(GEN9_3DSTATE_COMPONENT_PACKING | 3);
> + OUT_BATCH(1);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
>  
>   OUT_BATCH(GEN6_3DSTATE_VF_STATISTICS | 1 /* Enable */);
>  
> -- 
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx