All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state
@ 2015-07-31 15:27 Arun Siluvery
  2015-07-31 15:28 ` [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction Arun Siluvery
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Arun Siluvery @ 2015-07-31 15:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Cc: Ben Widawsky

A programming restriction exists for this instruction, atleast one component
of one valid vertex element must be enabled.

Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction
  2015-07-31 15:27 [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state Arun Siluvery
@ 2015-07-31 15:28 ` Arun Siluvery
  2015-08-05  1:37   ` Ben Widawsky
                     ` (2 more replies)
  2015-08-04 14:03 ` [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state Mika Kuoppala
  2015-08-05  1:36 ` Ben Widawsky
  2 siblings, 3 replies; 14+ messages in thread
From: Arun Siluvery @ 2015-07-31 15:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Atleast one component of one valid vertex element must be enabled.

Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_renderstate_gen9.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen9.c b/drivers/gpu/drm/i915/intel_renderstate_gen9.c
index 16a7ec2..fa86431 100644
--- a/drivers/gpu/drm/i915/intel_renderstate_gen9.c
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen9.c
@@ -873,7 +873,7 @@ static const u32 gen9_null_state_batch[] = {
 	0x00000000,
 	0x00000000,
 	0x78550003,
-	0x00000000,
+	0x00000001,
 	0x00000000,
 	0x00000000,
 	0x00000000,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state
  2015-07-31 15:27 [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state Arun Siluvery
  2015-07-31 15:28 ` [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction Arun Siluvery
@ 2015-08-04 14:03 ` Mika Kuoppala
  2015-08-05  1:36 ` Ben Widawsky
  2 siblings, 0 replies; 14+ messages in thread
From: Mika Kuoppala @ 2015-08-04 14:03 UTC (permalink / raw)
  To: Arun Siluvery, intel-gfx; +Cc: Cc: Ben Widawsky

Arun Siluvery <arun.siluvery@linux.intel.com> writes:

> A programming restriction exists for this instruction, atleast one component
> of one valid vertex element must be enabled.
>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state
  2015-07-31 15:27 [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state Arun Siluvery
  2015-07-31 15:28 ` [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction Arun Siluvery
  2015-08-04 14:03 ` [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state Mika Kuoppala
@ 2015-08-05  1:36 ` Ben Widawsky
  2015-08-05  8:13   ` Mika Kuoppala
  2015-08-13 12:30   ` Mika Kuoppala
  2 siblings, 2 replies; 14+ messages in thread
From: Ben Widawsky @ 2015-08-05  1:36 UTC (permalink / raw)
  To: Arun Siluvery; +Cc: intel-gfx

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 <benjamin.widawsky@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  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 <ben@bwidawsk.net>

I'm still more in favor of dropping the command altogether, that too would be:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction
  2015-07-31 15:28 ` [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction Arun Siluvery
@ 2015-08-05  1:37   ` Ben Widawsky
  2015-08-05  7:07   ` Mika Kuoppala
  2015-08-10 21:39   ` shuang.he
  2 siblings, 0 replies; 14+ messages in thread
From: Ben Widawsky @ 2015-08-05  1:37 UTC (permalink / raw)
  To: Arun Siluvery; +Cc: intel-gfx

On Fri, Jul 31, 2015 at 04:28:45PM +0100, Arun Siluvery wrote:
> Atleast one component of one valid vertex element must be enabled.
> 
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_renderstate_gen9.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen9.c b/drivers/gpu/drm/i915/intel_renderstate_gen9.c
> index 16a7ec2..fa86431 100644
> --- a/drivers/gpu/drm/i915/intel_renderstate_gen9.c
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen9.c
> @@ -873,7 +873,7 @@ static const u32 gen9_null_state_batch[] = {
>  	0x00000000,
>  	0x00000000,
>  	0x78550003,
> -	0x00000000,
> +	0x00000001,
>  	0x00000000,
>  	0x00000000,
>  	0x00000000,

Same as previous comment in patch 1, but silly programming is better than
definitely incorrect programming. With the comments from 1 addressed:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction
  2015-07-31 15:28 ` [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction Arun Siluvery
  2015-08-05  1:37   ` Ben Widawsky
@ 2015-08-05  7:07   ` Mika Kuoppala
  2015-08-10 21:39   ` shuang.he
  2 siblings, 0 replies; 14+ messages in thread
From: Mika Kuoppala @ 2015-08-05  7:07 UTC (permalink / raw)
  To: Arun Siluvery, intel-gfx; +Cc: Ben Widawsky

Arun Siluvery <arun.siluvery@linux.intel.com> writes:

> Atleast one component of one valid vertex element must be enabled.
>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---

Please resend the newly generated one when the IGT parts are merged.
That way we get the igt version info in the comment to match.

-Mika

>  drivers/gpu/drm/i915/intel_renderstate_gen9.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen9.c b/drivers/gpu/drm/i915/intel_renderstate_gen9.c
> index 16a7ec2..fa86431 100644
> --- a/drivers/gpu/drm/i915/intel_renderstate_gen9.c
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen9.c
> @@ -873,7 +873,7 @@ static const u32 gen9_null_state_batch[] = {
>  	0x00000000,
>  	0x00000000,
>  	0x78550003,
> -	0x00000000,
> +	0x00000001,
>  	0x00000000,
>  	0x00000000,
>  	0x00000000,
> -- 
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state
  2015-08-05  1:36 ` Ben Widawsky
@ 2015-08-05  8:13   ` Mika Kuoppala
  2015-08-05 18:17     ` Ben Widawsky
  2015-08-13 12:30   ` Mika Kuoppala
  1 sibling, 1 reply; 14+ messages in thread
From: Mika Kuoppala @ 2015-08-05  8:13 UTC (permalink / raw)
  To: Ben Widawsky, Arun Siluvery; +Cc: intel-gfx

Ben Widawsky <benjamin.widawsky@intel.com> 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 <benjamin.widawsky@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>> ---
>>  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 <ben@bwidawsk.net>
>
> I'm still more in favor of dropping the command altogether, that too would be:
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state
  2015-08-05  8:13   ` Mika Kuoppala
@ 2015-08-05 18:17     ` Ben Widawsky
  0 siblings, 0 replies; 14+ messages in thread
From: Ben Widawsky @ 2015-08-05 18:17 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Wed, Aug 05, 2015 at 11:13:46AM +0300, Mika Kuoppala wrote:
> Ben Widawsky <benjamin.widawsky@intel.com> 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 <benjamin.widawsky@intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> >> ---
> >>  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 <ben@bwidawsk.net>
> >
> > I'm still more in favor of dropping the command altogether, that too would be:
> > Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> 
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction
  2015-07-31 15:28 ` [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction Arun Siluvery
  2015-08-05  1:37   ` Ben Widawsky
  2015-08-05  7:07   ` Mika Kuoppala
@ 2015-08-10 21:39   ` shuang.he
  2 siblings, 0 replies; 14+ messages in thread
From: shuang.he @ 2015-08-10 21:39 UTC (permalink / raw)
  To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu,
	intel-gfx, arun.siluvery

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6912
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                 -1              302/302              301/302
SNB                                  315/315              315/315
IVB                                  336/336              336/336
BYT                 -2              283/283              281/283
HSW                                  378/378              378/378
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@kms_flip@flip-vs-dpms-interruptible      PASS(1)      DMESG_WARN(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
*BYT  igt@gem_persistent_relocs@interruptible      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state
  2015-08-05  1:36 ` Ben Widawsky
  2015-08-05  8:13   ` Mika Kuoppala
@ 2015-08-13 12:30   ` Mika Kuoppala
  2015-08-13 13:11     ` [PATCH] tools/null_state/gen9: Send all components " Mika Kuoppala
  1 sibling, 1 reply; 14+ messages in thread
From: Mika Kuoppala @ 2015-08-13 12:30 UTC (permalink / raw)
  To: Ben Widawsky, Arun Siluvery; +Cc: intel-gfx

Ben Widawsky <benjamin.widawsky@intel.com> 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 <benjamin.widawsky@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>> ---
>>  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 <ben@bwidawsk.net>
>

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@bwidawsk.net>
>
> -- 
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] tools/null_state/gen9: Send all components in VF state
  2015-08-13 12:30   ` Mika Kuoppala
@ 2015-08-13 13:11     ` Mika Kuoppala
  2015-08-17 16:54       ` Ben Widawsky
  2015-11-06 22:00       ` Chris Wilson
  0 siblings, 2 replies; 14+ messages in thread
From: Mika Kuoppala @ 2015-08-13 13:11 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Fix

commit 59cdc16b1a6f069f944ff17851a59edf8f72d45d
Author: Arun Siluvery <arun.siluvery@linux.intel.com>
Date:   Fri Jul 31 16:27:07 2015 +0100

    tools/null_state/gen9: Send atleast one valid component in VF state

to honor the Reviewed-by, send all four components as noted by
Ben in his review.

Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Arun Siluvery <arun.siluvery@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 tools/null_state_gen/intel_renderstate_gen9.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/null_state_gen/intel_renderstate_gen9.c b/tools/null_state_gen/intel_renderstate_gen9.c
index b3766ea..37bc675 100644
--- a/tools/null_state_gen/intel_renderstate_gen9.c
+++ b/tools/null_state_gen/intel_renderstate_gen9.c
@@ -442,7 +442,7 @@ int gen9_setup_null_render_state(struct intel_batchbuffer *batch)
 	gen8_emit_vertex_elements(batch);
 
 	OUT_BATCH(GEN9_3DSTATE_COMPONENT_PACKING | 3);
-	OUT_BATCH(1);
+	OUT_BATCH(0xf);
 	OUT_BATCH(0);
 	OUT_BATCH(0);
 	OUT_BATCH(0);
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools/null_state/gen9: Send all components in VF state
  2015-08-13 13:11     ` [PATCH] tools/null_state/gen9: Send all components " Mika Kuoppala
@ 2015-08-17 16:54       ` Ben Widawsky
  2015-11-06 22:00       ` Chris Wilson
  1 sibling, 0 replies; 14+ messages in thread
From: Ben Widawsky @ 2015-08-17 16:54 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Thu, Aug 13, 2015 at 04:11:00PM +0300, Mika Kuoppala wrote:
> Fix
> 
> commit 59cdc16b1a6f069f944ff17851a59edf8f72d45d
> Author: Arun Siluvery <arun.siluvery@linux.intel.com>
> Date:   Fri Jul 31 16:27:07 2015 +0100
> 
>     tools/null_state/gen9: Send atleast one valid component in VF state
> 
> to honor the Reviewed-by, send all four components as noted by
> Ben in his review.
> 
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Arun Siluvery <arun.siluvery@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Thanks. (STILL in favor of just dropping it :P )
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools/null_state/gen9: Send all components in VF state
  2015-08-13 13:11     ` [PATCH] tools/null_state/gen9: Send all components " Mika Kuoppala
  2015-08-17 16:54       ` Ben Widawsky
@ 2015-11-06 22:00       ` Chris Wilson
  2015-11-06 22:19         ` Ben Widawsky
  1 sibling, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2015-11-06 22:00 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx, Ben Widawsky

On Thu, Aug 13, 2015 at 04:11:00PM +0300, Mika Kuoppala wrote:
> Fix
> 
> commit 59cdc16b1a6f069f944ff17851a59edf8f72d45d
> Author: Arun Siluvery <arun.siluvery@linux.intel.com>
> Date:   Fri Jul 31 16:27:07 2015 +0100
> 
>     tools/null_state/gen9: Send atleast one valid component in VF state
> 
> to honor the Reviewed-by, send all four components as noted by
> Ben in his review.
> 
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Arun Siluvery <arun.siluvery@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Mika, did you forget something? The kernel patch?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools/null_state/gen9: Send all components in VF state
  2015-11-06 22:00       ` Chris Wilson
@ 2015-11-06 22:19         ` Ben Widawsky
  0 siblings, 0 replies; 14+ messages in thread
From: Ben Widawsky @ 2015-11-06 22:19 UTC (permalink / raw)
  To: Chris Wilson, Mika Kuoppala, intel-gfx

On Fri, Nov 06, 2015 at 10:00:19PM +0000, Chris Wilson wrote:
> On Thu, Aug 13, 2015 at 04:11:00PM +0300, Mika Kuoppala wrote:
> > Fix
> > 
> > commit 59cdc16b1a6f069f944ff17851a59edf8f72d45d
> > Author: Arun Siluvery <arun.siluvery@linux.intel.com>
> > Date:   Fri Jul 31 16:27:07 2015 +0100
> > 
> >     tools/null_state/gen9: Send atleast one valid component in VF state
> > 
> > to honor the Reviewed-by, send all four components as noted by
> > Ben in his review.
> > 
> > Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> > Cc: Arun Siluvery <arun.siluvery@linux.intel.com>
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Mika, did you forget something? The kernel patch?
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

When you guys have a stable I-G-T I'd strongly advocate for removing null state
on gen9 and seeing if it actually hurts anything.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-11-06 22:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-31 15:27 [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state Arun Siluvery
2015-07-31 15:28 ` [PATCH] drm/i915/gen9: Update null batch to follow VF state restriction Arun Siluvery
2015-08-05  1:37   ` Ben Widawsky
2015-08-05  7:07   ` Mika Kuoppala
2015-08-10 21:39   ` shuang.he
2015-08-04 14:03 ` [PATCH] tools/null_state/gen9: Send atleast one valid component in VF state Mika Kuoppala
2015-08-05  1:36 ` Ben Widawsky
2015-08-05  8:13   ` Mika Kuoppala
2015-08-05 18:17     ` Ben Widawsky
2015-08-13 12:30   ` Mika Kuoppala
2015-08-13 13:11     ` [PATCH] tools/null_state/gen9: Send all components " Mika Kuoppala
2015-08-17 16:54       ` Ben Widawsky
2015-11-06 22:00       ` Chris Wilson
2015-11-06 22:19         ` Ben Widawsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.