[Mesa-dev] [PATCH 2/2] genxml: Make BLEND_STATE command support variable length array.

Lionel Landwerlin lionel.g.landwerlin at intel.com
Sun Apr 9 19:18:07 UTC 2017


On 09/04/17 17:23, Jason Ekstrand wrote:
>
>
> On April 9, 2017 8:48:31 AM Lionel Landwerlin 
> <lionel.g.landwerlin at intel.com> wrote:
>
>> I have one suggestion at the bottom of the patch, otherwise :
>>
>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>>
>> On 07/04/17 17:52, Rafael Antognolli wrote:
>>> We need to emit BLEND_STATE, which size is 1 + 2 * nr_draw_buffers
>>> dwords (on gen8+), but the BLEND_STATE struct length is always 17. By
>>> marking it size 1, which is actually the size of the struct minus the
>>> BLEND_STATE_ENTRY's, we can emit a BLEND_STATE of variable number of
>>> entries.
>>>
>>> For gen6 and gen7 we set length to 0, since it only contains
>>> BLEND_STATE_ENTRY's, and no other data.
>>>
>>> With this change, we also change the code for blorp and anv to emit 
>>> only
>>> the needed BLEND_STATE_ENTRY's, instead of always emitting 16 dwords on
>>> gen6-7 and 17 dwords on gen8+.
>>>
>>> Signed-off-by: Rafael Antognolli <rafael.antognolli at intel.com>
>>> ---
>>>   src/intel/blorp/blorp_genX_exec.h | 35 ++++++++++++---------
>>>   src/intel/genxml/gen6.xml         |  4 +-
>>>   src/intel/genxml/gen7.xml         |  4 +-
>>>   src/intel/genxml/gen75.xml        |  4 +-
>>>   src/intel/genxml/gen8.xml         |  4 +-
>>>   src/intel/genxml/gen9.xml         |  4 +-
>>>   src/intel/vulkan/genX_pipeline.c  | 53 
>>> ++++++++++++++++----------------
>>>   7 files changed, 58 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/src/intel/blorp/blorp_genX_exec.h 
>>> b/src/intel/blorp/blorp_genX_exec.h
>>> index 3791462..fc1856f 100644
>>> --- a/src/intel/blorp/blorp_genX_exec.h
>>> +++ b/src/intel/blorp/blorp_genX_exec.h
>>> @@ -902,23 +902,30 @@ blorp_emit_blend_state(struct blorp_batch *batch,
>>>      struct GENX(BLEND_STATE) blend;
>>>      memset(&blend, 0, sizeof(blend));
>>>
>>> +   uint32_t offset;
>>> +   int size = GENX(BLEND_STATE_length) * 4;
>>> +   size += GENX(BLEND_STATE_ENTRY_length) * 4 * 
>>> params->num_draw_buffers;
>>> +   uint32_t *state = blorp_alloc_dynamic_state(batch, size, 64, 
>>> &offset);
>>> +   uint32_t *pos = state;
>>> +
>>> +   GENX(BLEND_STATE_pack)(NULL, pos, &blend);
>>> +   pos += GENX(BLEND_STATE_length);
>>> +
>>>      for (unsigned i = 0; i < params->num_draw_buffers; ++i) {
>>> -      blend.Entry[i].PreBlendColorClampEnable = true;
>>> -      blend.Entry[i].PostBlendColorClampEnable = true;
>>> -      blend.Entry[i].ColorClampRange = COLORCLAMP_RTFORMAT;
>>> -
>>> -      blend.Entry[i].WriteDisableRed = params->color_write_disable[0];
>>> -      blend.Entry[i].WriteDisableGreen = 
>>> params->color_write_disable[1];
>>> -      blend.Entry[i].WriteDisableBlue = 
>>> params->color_write_disable[2];
>>> -      blend.Entry[i].WriteDisableAlpha = 
>>> params->color_write_disable[3];
>>> +      struct GENX(BLEND_STATE_ENTRY) entry = { 0 };
>>> +      entry.PreBlendColorClampEnable = true;
>>> +      entry.PostBlendColorClampEnable = true;
>>> +      entry.ColorClampRange = COLORCLAMP_RTFORMAT;
>>> +
>>> +      entry.WriteDisableRed = params->color_write_disable[0];
>>> +      entry.WriteDisableGreen = params->color_write_disable[1];
>>> +      entry.WriteDisableBlue = params->color_write_disable[2];
>>> +      entry.WriteDisableAlpha = params->color_write_disable[3];
>>> +      GENX(BLEND_STATE_ENTRY_pack)(NULL, pos, &entry);
>>> +      pos += GENX(BLEND_STATE_ENTRY_length);
>>>      }
>>>
>>> -   uint32_t offset;
>>> -   void *state = blorp_alloc_dynamic_state(batch,
>>> - GENX(BLEND_STATE_length) * 4,
>>> -                                           64, &offset);
>>> -   GENX(BLEND_STATE_pack)(NULL, state, &blend);
>>> -   blorp_flush_range(batch, state, GENX(BLEND_STATE_length) * 4);
>>> +   blorp_flush_range(batch, state, size);
>>>
>>>   #if GEN_GEN >= 7
>>>      blorp_emit(batch, GENX(3DSTATE_BLEND_STATE_POINTERS), sp) {
>>> diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml
>>> index 5083f07..3059bfc 100644
>>> --- a/src/intel/genxml/gen6.xml
>>> +++ b/src/intel/genxml/gen6.xml
>>> @@ -452,8 +452,8 @@
>>>       <field name="Post-Blend Color Clamp Enable" start="32" 
>>> end="32" type="bool"/>
>>>     </struct>
>>>
>>> -  <struct name="BLEND_STATE" length="16">
>>> -    <group count="8" start="0" size="64">
>>> +  <struct name="BLEND_STATE" length="0">
>>> +    <group count="0" start="0" size="64">
>>>         <field name="Entry" start="0" end="63" 
>>> type="BLEND_STATE_ENTRY"/>
>>>       </group>
>>>     </struct>
>>> diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml
>>> index ada8f74..867a1d4 100644
>>> --- a/src/intel/genxml/gen7.xml
>>> +++ b/src/intel/genxml/gen7.xml
>>> @@ -507,8 +507,8 @@
>>>       <field name="Post-Blend Color Clamp Enable" start="32" 
>>> end="32" type="bool"/>
>>>     </struct>
>>>
>>> -  <struct name="BLEND_STATE" length="16">
>>> -    <group count="8" start="0" size="64">
>>> +  <struct name="BLEND_STATE" length="0">
>>> +    <group count="0" start="0" size="64">
>>>         <field name="Entry" start="0" end="63" 
>>> type="BLEND_STATE_ENTRY"/>
>>>       </group>
>>>     </struct>
>>> diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
>>> index 16d2d74..594e539 100644
>>> --- a/src/intel/genxml/gen75.xml
>>> +++ b/src/intel/genxml/gen75.xml
>>> @@ -517,8 +517,8 @@
>>>       <field name="Post-Blend Color Clamp Enable" start="32" 
>>> end="32" type="bool"/>
>>>     </struct>
>>>
>>> -  <struct name="BLEND_STATE" length="16">
>>> -    <group count="8" start="0" size="64">
>>> +  <struct name="BLEND_STATE" length="0">
>>> +    <group count="0" start="0" size="64">
>>>         <field name="Entry" start="0" end="63" 
>>> type="BLEND_STATE_ENTRY"/>
>>>       </group>
>>>     </struct>
>>> diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
>>> index 1390fe6..4985342 100644
>>> --- a/src/intel/genxml/gen8.xml
>>> +++ b/src/intel/genxml/gen8.xml
>>> @@ -546,7 +546,7 @@
>>>       <field name="Write Disable Blue" start="0" end="0" type="bool"/>
>>>     </struct>
>>>
>>> -  <struct name="BLEND_STATE" length="17">
>>> +  <struct name="BLEND_STATE" length="1">
>>>       <field name="Alpha To Coverage Enable" start="31" end="31" 
>>> type="bool"/>
>>>       <field name="Independent Alpha Blend Enable" start="30" 
>>> end="30" type="bool"/>
>>>       <field name="Alpha To One Enable" start="29" end="29" 
>>> type="bool"/>
>>> @@ -556,7 +556,7 @@
>>>       <field name="Color Dither Enable" start="23" end="23" 
>>> type="bool"/>
>>>       <field name="X Dither Offset" start="21" end="22" type="uint"/>
>>>       <field name="Y Dither Offset" start="19" end="20" type="uint"/>
>>> -    <group count="8" start="32" size="64">
>>> +    <group count="0" start="32" size="64">
>>>         <field name="Entry" start="0" end="63" 
>>> type="BLEND_STATE_ENTRY"/>
>>>       </group>
>>>     </struct>
>>> diff --git a/src/intel/genxml/gen9.xml b/src/intel/genxml/gen9.xml
>>> index 4bf0fb6..a620e78 100644
>>> --- a/src/intel/genxml/gen9.xml
>>> +++ b/src/intel/genxml/gen9.xml
>>> @@ -555,7 +555,7 @@
>>>       <field name="Write Disable Blue" start="0" end="0" type="bool"/>
>>>     </struct>
>>>
>>> -  <struct name="BLEND_STATE" length="17">
>>> +  <struct name="BLEND_STATE" length="1">
>>>       <field name="Alpha To Coverage Enable" start="31" end="31" 
>>> type="bool"/>
>>>       <field name="Independent Alpha Blend Enable" start="30" 
>>> end="30" type="bool"/>
>>>       <field name="Alpha To One Enable" start="29" end="29" 
>>> type="bool"/>
>>> @@ -565,7 +565,7 @@
>>>       <field name="Color Dither Enable" start="23" end="23" 
>>> type="bool"/>
>>>       <field name="X Dither Offset" start="21" end="22" type="uint"/>
>>>       <field name="Y Dither Offset" start="19" end="20" type="uint"/>
>>> -    <group count="8" start="32" size="64">
>>> +    <group count="0" start="32" size="64">
>>>         <field name="Entry" start="0" end="63" 
>>> type="BLEND_STATE_ENTRY"/>
>>>       </group>
>>>     </struct>
>>> diff --git a/src/intel/vulkan/genX_pipeline.c 
>>> b/src/intel/vulkan/genX_pipeline.c
>>> index 3fd1333..894d584 100644
>>> --- a/src/intel/vulkan/genX_pipeline.c
>>> +++ b/src/intel/vulkan/genX_pipeline.c
>>> @@ -862,28 +862,14 @@ emit_cb_state(struct anv_pipeline *pipeline,
>>>   {
>>>      struct anv_device *device = pipeline->device;
>>>
>>> -   const uint32_t num_dwords = GENX(BLEND_STATE_length);
>>> -   pipeline->blend_state =
>>> - anv_state_pool_alloc(&device->dynamic_state_pool, num_dwords * 4, 
>>> 64);
>>>
>>>      struct GENX(BLEND_STATE) blend_state = {
>>>   #if GEN_GEN >= 8
>>>         .AlphaToCoverageEnable = ms_info && 
>>> ms_info->alphaToCoverageEnable,
>>>         .AlphaToOneEnable = ms_info && ms_info->alphaToOneEnable,
>>> -#else
>>> -      /* Make sure it gets zeroed */
>>> -      .Entry = { { 0, }, },
>>>   #endif
>>>      };
>>>
>>> -   /* Default everything to disabled */
>>> -   for (uint32_t i = 0; i < 8; i++) {
>>> -      blend_state.Entry[i].WriteDisableAlpha = true;
>>> -      blend_state.Entry[i].WriteDisableRed = true;
>>> -      blend_state.Entry[i].WriteDisableGreen = true;
>>> -      blend_state.Entry[i].WriteDisableBlue = true;
>>> -   }
>>> -
>>>      uint32_t surface_count = 0;
>>>      struct anv_pipeline_bind_map *map;
>>>      if (anv_pipeline_has_stage(pipeline, MESA_SHADER_FRAGMENT)) {
>>> @@ -891,7 +877,17 @@ emit_cb_state(struct anv_pipeline *pipeline,
>>>         surface_count = map->surface_count;
>>>      }
>>>
>>> +   const uint32_t num_dwords = GENX(BLEND_STATE_length) +
>>> +      GENX(BLEND_STATE_ENTRY_length) * surface_count;
>>> +   pipeline->blend_state =
>>> + anv_state_pool_alloc(&device->dynamic_state_pool, num_dwords * 4, 
>>> 64);
>>> +
>>>      bool has_writeable_rt = false;
>>> +   uint32_t *state_pos = pipeline->blend_state.map;
>>> +   state_pos += GENX(BLEND_STATE_length);
>>> +#if GEN_GEN >= 8
>>> +   struct GENX(BLEND_STATE_ENTRY) bs0 = { 0 };
>>> +#endif
>>>      for (unsigned i = 0; i < surface_count; i++) {
>>>         struct anv_pipeline_binding *binding = 
>>> &map->surface_to_descriptor[i];
>>>
>>> @@ -909,7 +905,7 @@ emit_cb_state(struct anv_pipeline *pipeline,
>>>         const VkPipelineColorBlendAttachmentState *a =
>>>            &info->pAttachments[binding->index];
>>>
>>> -      blend_state.Entry[i] = (struct GENX(BLEND_STATE_ENTRY)) {
>>> +      struct GENX(BLEND_STATE_ENTRY) entry = { 0,
>>>   #if GEN_GEN < 8
>>>            .AlphaToCoverageEnable = ms_info && 
>>> ms_info->alphaToCoverageEnable,
>>>            .AlphaToOneEnable = ms_info && ms_info->alphaToOneEnable,
>>> @@ -938,7 +934,7 @@ emit_cb_state(struct anv_pipeline *pipeline,
>>>   #if GEN_GEN >= 8
>>>            blend_state.IndependentAlphaBlendEnable = true;
>>>   #else
>>> -         blend_state.Entry[i].IndependentAlphaBlendEnable = true;
>>> +         entry.IndependentAlphaBlendEnable = true;
>>>   #endif
>>>         }
>>>
>>> @@ -953,26 +949,31 @@ emit_cb_state(struct anv_pipeline *pipeline,
>>>          */
>>>         if (a->colorBlendOp == VK_BLEND_OP_MIN ||
>>>             a->colorBlendOp == VK_BLEND_OP_MAX) {
>>> -         blend_state.Entry[i].SourceBlendFactor = BLENDFACTOR_ONE;
>>> -         blend_state.Entry[i].DestinationBlendFactor = 
>>> BLENDFACTOR_ONE;
>>> +         entry.SourceBlendFactor = BLENDFACTOR_ONE;
>>> +         entry.DestinationBlendFactor = BLENDFACTOR_ONE;
>>>         }
>>>         if (a->alphaBlendOp == VK_BLEND_OP_MIN ||
>>>             a->alphaBlendOp == VK_BLEND_OP_MAX) {
>>> -         blend_state.Entry[i].SourceAlphaBlendFactor = 
>>> BLENDFACTOR_ONE;
>>> -         blend_state.Entry[i].DestinationAlphaBlendFactor = 
>>> BLENDFACTOR_ONE;
>>> +         entry.SourceAlphaBlendFactor = BLENDFACTOR_ONE;
>>> +         entry.DestinationAlphaBlendFactor = BLENDFACTOR_ONE;
>>>         }
>>> +      GENX(BLEND_STATE_ENTRY_pack)(NULL, state_pos, &entry);
>>> +      state_pos += GENX(BLEND_STATE_ENTRY_length);
>>> +#if GEN_GEN >= 8
>>> +      if (i == 0)
>>> +         bs0 = entry;
>>> +#endif
>>>      }
>>>
>>>   #if GEN_GEN >= 8
>>> -   struct GENX(BLEND_STATE_ENTRY) *bs0 = &blend_state.Entry[0];
>>
>> You could keep a pointer here and initialize it to :
>>
>> pipeline->blend_state.map + GENX(BLEND_STATE_length);
>
> Careful.  map is a void* and _length is in dwords.

Ah Thanks!
I always get confused...

>
>>> anv_batch_emit(&pipeline->batch, GENX(3DSTATE_PS_BLEND), blend) {
>>>         blend.AlphaToCoverageEnable         = 
>>> blend_state.AlphaToCoverageEnable;
>>>         blend.HasWriteableRT                = has_writeable_rt;
>>> -      blend.ColorBufferBlendEnable        = 
>>> bs0->ColorBufferBlendEnable;
>>> -      blend.SourceAlphaBlendFactor        = 
>>> bs0->SourceAlphaBlendFactor;
>>> -      blend.DestinationAlphaBlendFactor   = 
>>> bs0->DestinationAlphaBlendFactor;
>>> -      blend.SourceBlendFactor             = bs0->SourceBlendFactor;
>>> -      blend.DestinationBlendFactor        = 
>>> bs0->DestinationBlendFactor;
>>> +      blend.ColorBufferBlendEnable        = 
>>> bs0.ColorBufferBlendEnable;
>>> +      blend.SourceAlphaBlendFactor        = 
>>> bs0.SourceAlphaBlendFactor;
>>> +      blend.DestinationAlphaBlendFactor   = 
>>> bs0.DestinationAlphaBlendFactor;
>>> +      blend.SourceBlendFactor             = bs0.SourceBlendFactor;
>>> +      blend.DestinationBlendFactor        = 
>>> bs0.DestinationBlendFactor;
>>>         blend.AlphaTestEnable               = false;
>>>         blend.IndependentAlphaBlendEnable   =
>>>            blend_state.IndependentAlphaBlendEnable;
>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev




More information about the mesa-dev mailing list