[Mesa-dev] [PATCH 2/2] genxml: Make BLEND_STATE command support variable length array.
Jason Ekstrand
jason at jlekstrand.net
Wed Apr 12 17:45:58 UTC 2017
On Fri, Apr 7, 2017 at 9:52 AM, Rafael Antognolli <
rafael.antognolli at intel.com> 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];
>
Might be a tiny bit nicer to use a designated initializer here.
> + 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;
>
I think there was some reason why I filled out all 8 of them but I can't
remember what it was. If this passes the Vulkan CTS, it's probably fine.
> - }
> -
> 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,
>
You don't need the extra 0 there. Designated initializers (as used below)
will take care of initializing the rest to 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
>
This is a bit annoying but I don't see a good way around it.
> }
>
> #if GEN_GEN >= 8
> - struct GENX(BLEND_STATE_ENTRY) *bs0 = &blend_state.Entry[0];
> 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;
> --
> git-series 0.9.1
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170412/6ebba8e9/attachment-0001.html>
More information about the mesa-dev
mailing list