On Sun, Apr 09, 2017 at 08:18:07PM +0100, Lionel Landwerlin wrote:
> On 09/04/17 17:23, Jason Ekstrand wrote:
> >
> >
> > On April 9, 2017 8:48:31 AM Lionel Landwerlin
> > <lionel.g.landwer...@intel.com> wrote:
> >
> > > I have one suggestion at the bottom of the patch, otherwise :
> > >
> > > Reviewed-by: Lionel Landwerlin <lionel.g.landwer...@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.antogno...@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...
Hmm... nice idea, but pipeline->blend_state.map contains the "packed"
version of the struct, so I can't access things like
bs0->has_writeable_rt.
I would have to do bit operations, but I'm not
sure it's worth the effort.
> >
> > > > 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@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
