Re: [Mesa-dev] [Mesa-stable] [PATCH] intel: decoder: unify MI_BB_START field naming
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
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
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
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