Re: [Mesa-dev] [Mesa-stable] [PATCH] intel: decoder: unify MI_BB_START field naming

2018-08-30 Thread Lionel Landwerlin

On 30/08/2018 16:23, Dylan Baker wrote:

Quoting Lionel Landwerlin (2018-08-30 06:58:39)

On 28/08/2018 17:01, Dylan Baker wrote:

Quoting Lionel Landwerlin (2018-08-28 02:39:58)

Yes, I think so. You asked on another commit too, both are related and
this depends on other commits from Jason.

Here is a list in order of cherry picking :

commit f430a37fa75f534c3a114b0ec546fa14f05f5da1
Author: Lionel Landwerlin 
Date:   Tue Aug 14 11:22:12 2018 +0100

       intel: decoder: unify MI_BB_START field naming

commit 2abd7ae189135eb5a1f530a3a1c9412d3a7e238d
Author: Jason Ekstrand 
Date:   Fri Aug 24 15:23:04 2018 -0500

       intel/decoder: Clean up field iteration and fix sub-dword fields

commit cbd4bc1346f7397242e157bb66099b950a8c5643
Author: Jason Ekstrand 
Date:   Fri Aug 24 16:04:03 2018 -0500

       intel/batch_decoder: Fix dynamic state printing

commit 70de31d0c106f58d6b7e6d5b79b8d90c1c112a3b
Author: Jason Ekstrand 
Date:   Fri Aug 24 16:05:08 2018 -0500

       intel/batch_decoder: Print blend states properly


commit 440a988bd1478bb33dafcbb8575473bc643ae383
Author: Lionel Landwerlin 
Date:   Sat Aug 25 18:22:00 2018 +0100

       intel: decoder: handle 0 sized structs

Thanks,

-
Lionel

On 27/08/2018 22:20, Andres Gomez wrote:

Lionel, should we also include this in the stable queues ?

On Tue, 2018-08-14 at 11:26 +0100, Lionel Landwerlin wrote:

The batch decoder looks for a field with a particular name to decide
whether an MI_BB_START leads into a second batch buffer level. Because
the names are different between Gen7.5/8 and the newer generation we
fail that test and keep on reading (invalid) instructions.

Signed-off-by: Lionel Landwerlin 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
---
src/intel/genxml/gen75.xml | 6 +++---
src/intel/genxml/gen8.xml  | 6 +++---
src/intel/vulkan/anv_batch_chain.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 5b01fd45400..dfc3d891498 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -2314,9 +2314,9 @@
  


-
-  
-  
+
+  
+  



diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
index 4ed41d15612..330366b7ed0 100644
--- a/src/intel/genxml/gen8.xml
+++ b/src/intel/genxml/gen8.xml
@@ -2553,9 +2553,9 @@
  


-
-  
-  
+
+  
+  



diff --git a/src/intel/vulkan/anv_batch_chain.c 
b/src/intel/vulkan/anv_batch_chain.c
index c47a81c8a4d..0f7c8325ea4 100644
--- a/src/intel/vulkan/anv_batch_chain.c
+++ b/src/intel/vulkan/anv_batch_chain.c
@@ -531,7 +531,7 @@ emit_batch_buffer_start(struct anv_cmd_buffer *cmd_buffer,
   anv_batch_emit(_buffer->batch, GEN8_MI_BATCH_BUFFER_START, bbs) {
  bbs.DWordLength   = cmd_buffer->device->info.gen < 8 ?
  gen7_length : gen8_length;
-  bbs._2ndLevelBatchBuffer  = _1stlevelbatch;
+  bbs.SecondLevelBatchBuffer= Firstlevelbatch;
  bbs.AddressSpaceIndicator = ASI_PPGTT;
  bbs.BatchBufferStartAddress   = (struct anv_address) { bo, offset };
   }

Hi Lionel,

Only patches 1 and 3 of that list apply cleanly to the 18.1 branch, it looks
like maybe I need a few more patches for things to apply cleanly?

Dylan

Yeah... Looks like this might pull in the world...
Thinking about it again, how much do we need INTEL_DEBUG=batch fixed in
stable?

-
Lionel

If it applies cleanly I think it's fine, but I don't think it's worth it if it's
going to require a lot of other patches. So just drop this for 18.1?

Dylan



Yes, let's just drop it.

Thanks,

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] intel: decoder: unify MI_BB_START field naming

2018-08-30 Thread Dylan Baker
Quoting Lionel Landwerlin (2018-08-30 06:58:39)
> On 28/08/2018 17:01, Dylan Baker wrote:
> > Quoting Lionel Landwerlin (2018-08-28 02:39:58)
> >> Yes, I think so. You asked on another commit too, both are related and
> >> this depends on other commits from Jason.
> >>
> >> Here is a list in order of cherry picking :
> >>
> >> commit f430a37fa75f534c3a114b0ec546fa14f05f5da1
> >> Author: Lionel Landwerlin 
> >> Date:   Tue Aug 14 11:22:12 2018 +0100
> >>
> >>       intel: decoder: unify MI_BB_START field naming
> >>
> >> commit 2abd7ae189135eb5a1f530a3a1c9412d3a7e238d
> >> Author: Jason Ekstrand 
> >> Date:   Fri Aug 24 15:23:04 2018 -0500
> >>
> >>       intel/decoder: Clean up field iteration and fix sub-dword fields
> >>
> >> commit cbd4bc1346f7397242e157bb66099b950a8c5643
> >> Author: Jason Ekstrand 
> >> Date:   Fri Aug 24 16:04:03 2018 -0500
> >>
> >>       intel/batch_decoder: Fix dynamic state printing
> >>
> >> commit 70de31d0c106f58d6b7e6d5b79b8d90c1c112a3b
> >> Author: Jason Ekstrand 
> >> Date:   Fri Aug 24 16:05:08 2018 -0500
> >>
> >>       intel/batch_decoder: Print blend states properly
> >>
> >>
> >> commit 440a988bd1478bb33dafcbb8575473bc643ae383
> >> Author: Lionel Landwerlin 
> >> Date:   Sat Aug 25 18:22:00 2018 +0100
> >>
> >>       intel: decoder: handle 0 sized structs
> >>
> >> Thanks,
> >>
> >> -
> >> Lionel
> >>
> >> On 27/08/2018 22:20, Andres Gomez wrote:
> >>> Lionel, should we also include this in the stable queues ?
> >>>
> >>> On Tue, 2018-08-14 at 11:26 +0100, Lionel Landwerlin wrote:
>  The batch decoder looks for a field with a particular name to decide
>  whether an MI_BB_START leads into a second batch buffer level. Because
>  the names are different between Gen7.5/8 and the newer generation we
>  fail that test and keep on reading (invalid) instructions.
> 
>  Signed-off-by: Lionel Landwerlin 
>  Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
>  ---
> src/intel/genxml/gen75.xml | 6 +++---
> src/intel/genxml/gen8.xml  | 6 +++---
> src/intel/vulkan/anv_batch_chain.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
> 
>  diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
>  index 5b01fd45400..dfc3d891498 100644
>  --- a/src/intel/genxml/gen75.xml
>  +++ b/src/intel/genxml/gen75.xml
>  @@ -2314,9 +2314,9 @@
>   
>   default="0"/>
>   default="49"/>
>  -  type="uint">
>  -  
>  -  
>  +  type="uint">
>  +  
>  +  
> 
> 
> 
>  diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
>  index 4ed41d15612..330366b7ed0 100644
>  --- a/src/intel/genxml/gen8.xml
>  +++ b/src/intel/genxml/gen8.xml
>  @@ -2553,9 +2553,9 @@
>   
>   default="0"/>
>   default="49"/>
>  -  type="uint">
>  -  
>  -  
>  +  type="uint">
>  +  
>  +  
> 
> 
> 
>  diff --git a/src/intel/vulkan/anv_batch_chain.c 
>  b/src/intel/vulkan/anv_batch_chain.c
>  index c47a81c8a4d..0f7c8325ea4 100644
>  --- a/src/intel/vulkan/anv_batch_chain.c
>  +++ b/src/intel/vulkan/anv_batch_chain.c
>  @@ -531,7 +531,7 @@ emit_batch_buffer_start(struct anv_cmd_buffer 
>  *cmd_buffer,
>    anv_batch_emit(_buffer->batch, GEN8_MI_BATCH_BUFFER_START, 
>  bbs) {
>   bbs.DWordLength   = cmd_buffer->device->info.gen < 
>  8 ?
>   gen7_length : gen8_length;
>  -  bbs._2ndLevelBatchBuffer  = _1stlevelbatch;
>  +  bbs.SecondLevelBatchBuffer= Firstlevelbatch;
>   bbs.AddressSpaceIndicator = ASI_PPGTT;
>   bbs.BatchBufferStartAddress   = (struct anv_address) { bo, 
>  offset };
>    }
> > Hi Lionel,
> >
> > Only patches 1 and 3 of that list apply cleanly to the 18.1 branch, it looks
> > like maybe I need a few more patches for things to apply cleanly?
> >
> > Dylan
> 
> Yeah... Looks like this might pull in the world...
> Thinking about it again, how much do we need INTEL_DEBUG=batch fixed in 
> stable?
> 
> -
> Lionel

If it applies cleanly I think it's fine, but I don't think it's worth it if it's
going to require a lot of other patches. So just drop this for 18.1?

Dylan


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


Re: [Mesa-dev] [Mesa-stable] [PATCH] intel: decoder: unify MI_BB_START field naming

2018-08-30 Thread Lionel Landwerlin

On 28/08/2018 17:01, Dylan Baker wrote:

Quoting Lionel Landwerlin (2018-08-28 02:39:58)

Yes, I think so. You asked on another commit too, both are related and
this depends on other commits from Jason.

Here is a list in order of cherry picking :

commit f430a37fa75f534c3a114b0ec546fa14f05f5da1
Author: Lionel Landwerlin 
Date:   Tue Aug 14 11:22:12 2018 +0100

      intel: decoder: unify MI_BB_START field naming

commit 2abd7ae189135eb5a1f530a3a1c9412d3a7e238d
Author: Jason Ekstrand 
Date:   Fri Aug 24 15:23:04 2018 -0500

      intel/decoder: Clean up field iteration and fix sub-dword fields

commit cbd4bc1346f7397242e157bb66099b950a8c5643
Author: Jason Ekstrand 
Date:   Fri Aug 24 16:04:03 2018 -0500

      intel/batch_decoder: Fix dynamic state printing

commit 70de31d0c106f58d6b7e6d5b79b8d90c1c112a3b
Author: Jason Ekstrand 
Date:   Fri Aug 24 16:05:08 2018 -0500

      intel/batch_decoder: Print blend states properly


commit 440a988bd1478bb33dafcbb8575473bc643ae383
Author: Lionel Landwerlin 
Date:   Sat Aug 25 18:22:00 2018 +0100

      intel: decoder: handle 0 sized structs

Thanks,

-
Lionel

On 27/08/2018 22:20, Andres Gomez wrote:

Lionel, should we also include this in the stable queues ?

On Tue, 2018-08-14 at 11:26 +0100, Lionel Landwerlin wrote:

The batch decoder looks for a field with a particular name to decide
whether an MI_BB_START leads into a second batch buffer level. Because
the names are different between Gen7.5/8 and the newer generation we
fail that test and keep on reading (invalid) instructions.

Signed-off-by: Lionel Landwerlin 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
---
   src/intel/genxml/gen75.xml | 6 +++---
   src/intel/genxml/gen8.xml  | 6 +++---
   src/intel/vulkan/anv_batch_chain.c | 2 +-
   3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 5b01fd45400..dfc3d891498 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -2314,9 +2314,9 @@
 
   
   
-
-  
-  
+
+  
+  
   
   
   
diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
index 4ed41d15612..330366b7ed0 100644
--- a/src/intel/genxml/gen8.xml
+++ b/src/intel/genxml/gen8.xml
@@ -2553,9 +2553,9 @@
 
   
   
-
-  
-  
+
+  
+  
   
   
   
diff --git a/src/intel/vulkan/anv_batch_chain.c 
b/src/intel/vulkan/anv_batch_chain.c
index c47a81c8a4d..0f7c8325ea4 100644
--- a/src/intel/vulkan/anv_batch_chain.c
+++ b/src/intel/vulkan/anv_batch_chain.c
@@ -531,7 +531,7 @@ emit_batch_buffer_start(struct anv_cmd_buffer *cmd_buffer,
  anv_batch_emit(_buffer->batch, GEN8_MI_BATCH_BUFFER_START, bbs) {
 bbs.DWordLength   = cmd_buffer->device->info.gen < 8 ?
 gen7_length : gen8_length;
-  bbs._2ndLevelBatchBuffer  = _1stlevelbatch;
+  bbs.SecondLevelBatchBuffer= Firstlevelbatch;
 bbs.AddressSpaceIndicator = ASI_PPGTT;
 bbs.BatchBufferStartAddress   = (struct anv_address) { bo, offset };
  }

Hi Lionel,

Only patches 1 and 3 of that list apply cleanly to the 18.1 branch, it looks
like maybe I need a few more patches for things to apply cleanly?

Dylan


Yeah... Looks like this might pull in the world...
Thinking about it again, how much do we need INTEL_DEBUG=batch fixed in 
stable?


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


Re: [Mesa-dev] [Mesa-stable] [PATCH] intel: decoder: unify MI_BB_START field naming

2018-08-28 Thread Dylan Baker
Quoting Lionel Landwerlin (2018-08-28 02:39:58)
> Yes, I think so. You asked on another commit too, both are related and 
> this depends on other commits from Jason.
> 
> Here is a list in order of cherry picking :
> 
> commit f430a37fa75f534c3a114b0ec546fa14f05f5da1
> Author: Lionel Landwerlin 
> Date:   Tue Aug 14 11:22:12 2018 +0100
> 
>      intel: decoder: unify MI_BB_START field naming
> 
> commit 2abd7ae189135eb5a1f530a3a1c9412d3a7e238d
> Author: Jason Ekstrand 
> Date:   Fri Aug 24 15:23:04 2018 -0500
> 
>      intel/decoder: Clean up field iteration and fix sub-dword fields
> 
> commit cbd4bc1346f7397242e157bb66099b950a8c5643
> Author: Jason Ekstrand 
> Date:   Fri Aug 24 16:04:03 2018 -0500
> 
>      intel/batch_decoder: Fix dynamic state printing
> 
> commit 70de31d0c106f58d6b7e6d5b79b8d90c1c112a3b
> Author: Jason Ekstrand 
> Date:   Fri Aug 24 16:05:08 2018 -0500
> 
>      intel/batch_decoder: Print blend states properly
> 
> 
> commit 440a988bd1478bb33dafcbb8575473bc643ae383
> Author: Lionel Landwerlin 
> Date:   Sat Aug 25 18:22:00 2018 +0100
> 
>      intel: decoder: handle 0 sized structs
> 
> Thanks,
> 
> -
> Lionel
> 
> On 27/08/2018 22:20, Andres Gomez wrote:
> > Lionel, should we also include this in the stable queues ?
> >
> > On Tue, 2018-08-14 at 11:26 +0100, Lionel Landwerlin wrote:
> >> The batch decoder looks for a field with a particular name to decide
> >> whether an MI_BB_START leads into a second batch buffer level. Because
> >> the names are different between Gen7.5/8 and the newer generation we
> >> fail that test and keep on reading (invalid) instructions.
> >>
> >> Signed-off-by: Lionel Landwerlin 
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
> >> ---
> >>   src/intel/genxml/gen75.xml | 6 +++---
> >>   src/intel/genxml/gen8.xml  | 6 +++---
> >>   src/intel/vulkan/anv_batch_chain.c | 2 +-
> >>   3 files changed, 7 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
> >> index 5b01fd45400..dfc3d891498 100644
> >> --- a/src/intel/genxml/gen75.xml
> >> +++ b/src/intel/genxml/gen75.xml
> >> @@ -2314,9 +2314,9 @@
> >> 
> >>>> default="0"/>
> >>>> default="49"/>
> >> -
> >> -  
> >> -  
> >> + >> type="uint">
> >> +  
> >> +  
> >>   
> >>   
> >>   
> >> diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
> >> index 4ed41d15612..330366b7ed0 100644
> >> --- a/src/intel/genxml/gen8.xml
> >> +++ b/src/intel/genxml/gen8.xml
> >> @@ -2553,9 +2553,9 @@
> >> 
> >>>> default="0"/>
> >>>> default="49"/>
> >> -
> >> -  
> >> -  
> >> + >> type="uint">
> >> +  
> >> +  
> >>   
> >>   
> >>   
> >> diff --git a/src/intel/vulkan/anv_batch_chain.c 
> >> b/src/intel/vulkan/anv_batch_chain.c
> >> index c47a81c8a4d..0f7c8325ea4 100644
> >> --- a/src/intel/vulkan/anv_batch_chain.c
> >> +++ b/src/intel/vulkan/anv_batch_chain.c
> >> @@ -531,7 +531,7 @@ emit_batch_buffer_start(struct anv_cmd_buffer 
> >> *cmd_buffer,
> >>  anv_batch_emit(_buffer->batch, GEN8_MI_BATCH_BUFFER_START, bbs) {
> >> bbs.DWordLength   = cmd_buffer->device->info.gen < 8 ?
> >> gen7_length : gen8_length;
> >> -  bbs._2ndLevelBatchBuffer  = _1stlevelbatch;
> >> +  bbs.SecondLevelBatchBuffer= Firstlevelbatch;
> >> bbs.AddressSpaceIndicator = ASI_PPGTT;
> >> bbs.BatchBufferStartAddress   = (struct anv_address) { bo, offset 
> >> };
> >>  }

Hi Lionel,

Only patches 1 and 3 of that list apply cleanly to the 18.1 branch, it looks
like maybe I need a few more patches for things to apply cleanly?

Dylan


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