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

Thu Aug 30 15:31:58 UTC 2018

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 <lionel.g.landwerlin at intel.com>
>>>> Date:   Tue Aug 14 11:22:12 2018 +0100
>>>>
>>>>        intel: decoder: unify MI_BB_START field naming
>>>>
>>>> commit 2abd7ae189135eb5a1f530a3a1c9412d3a7e238d
>>>> Author: Jason Ekstrand <jason.ekstrand at intel.com>
>>>> 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 <jason.ekstrand at intel.com>
>>>> Date:   Fri Aug 24 16:04:03 2018 -0500
>>>>
>>>>        intel/batch_decoder: Fix dynamic state printing
>>>>
>>>> commit 70de31d0c106f58d6b7e6d5b79b8d90c1c112a3b
>>>> Author: Jason Ekstrand <jason.ekstrand at intel.com>
>>>> Date:   Fri Aug 24 16:05:08 2018 -0500
>>>>
>>>>        intel/batch_decoder: Print blend states properly
>>>>
>>>>
>>>> commit 440a988bd1478bb33dafcbb8575473bc643ae383
>>>> Author: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>>>> 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 <lionel.g.landwerlin at intel.com>
>>>>>> 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 @@
>>>>>>       <instruction name="MI_BATCH_BUFFER_START" bias="2" length="2">
>>>>>>         <field name="Command Type" start="29" end="31" type="uint" default="0"/>
>>>>>>         <field name="MI Command Opcode" start="23" end="28" type="uint" default="49"/>
>>>>>> -    <field name="2nd Level Batch Buffer" start="22" end="22" type="uint">
>>>>>> -      <value name="1st level batch" value="0"/>
>>>>>> -      <value name="2nd level batch" value="1"/>
>>>>>> +    <field name="Second Level Batch Buffer" start="22" end="22" type="uint">
>>>>>> +      <value name="First level batch" value="0"/>
>>>>>> +      <value name="Second level batch" value="1"/>
>>>>>>         </field>
>>>>>>         <field name="Add Offset Enable" start="16" end="16" type="bool"/>
>>>>>>         <field name="Predication Enable" start="15" end="15" type="bool"/>
>>>>>> 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 @@
>>>>>>       <instruction name="MI_BATCH_BUFFER_START" bias="2" length="3">
>>>>>>         <field name="Command Type" start="29" end="31" type="uint" default="0"/>
>>>>>>         <field name="MI Command Opcode" start="23" end="28" type="uint" default="49"/>
>>>>>> -    <field name="2nd Level Batch Buffer" start="22" end="22" type="uint">
>>>>>> -      <value name="1st level batch" value="0"/>
>>>>>> -      <value name="2nd level batch" value="1"/>
>>>>>> +    <field name="Second Level Batch Buffer" start="22" end="22" type="uint">
>>>>>> +      <value name="First level batch" value="0"/>
>>>>>> +      <value name="Second level batch" value="1"/>
>>>>>>         </field>
>>>>>>         <field name="Add Offset Enable" start="16" end="16" type="bool"/>
>>>>>>         <field name="Predication Enable" start="15" end="15" type="bool"/>
>>>>>> 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(&cmd_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