[Mesa-dev] [PATCH 08/20] anv/pipeline: Unify 3DSTATE_GS emission
Timothy Arceri
timothy.arceri at collabora.com
Mon Nov 14 21:58:31 UTC 2016
On Mon, 2016-11-14 at 08:46 -0800, Jason Ekstrand wrote:
>
>
> On Sat, Nov 12, 2016 at 2:54 PM, Timothy Arceri <timothy.arceri at colla
> bora.com> wrote:
> > On Sat, 2016-11-12 at 13:34 -0800, Jason Ekstrand wrote:
> >
> > I have two questions and two suggestions below.
> >
> > With the suggestions addressed and assuming the answer to both
> > questions is yes, Patch 7-8 are:
> >
> > Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>
> >
> > > ---
> > > src/intel/vulkan/gen7_pipeline.c | 48 +------------------
> > ----
> > > src/intel/vulkan/gen8_pipeline.c | 62 +------------------
> > ----
> > > ------
> > > src/intel/vulkan/genX_pipeline_util.h | 73
> > > +++++++++++++++++++++++++++++++++++
> > > 3 files changed, 75 insertions(+), 108 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/gen7_pipeline.c
> > > b/src/intel/vulkan/gen7_pipeline.c
> > > index e604c25..52577f5 100644
> > > --- a/src/intel/vulkan/gen7_pipeline.c
> > > +++ b/src/intel/vulkan/gen7_pipeline.c
> > > @@ -105,53 +105,7 @@ genX(graphics_pipeline_create)(
> > > #endif
> > >
> > > emit_3dstate_vs(pipeline);
> > > -
> > > - const struct brw_gs_prog_data *gs_prog_data =
> > > get_gs_prog_data(pipeline);
> > > -
> > > - if (!anv_pipeline_has_stage(pipeline, MESA_SHADER_GEOMETRY))
> > {
> > > - anv_batch_emit(&pipeline->batch, GENX(3DSTATE_GS), gs);
> > > - } else {
> > > - const struct anv_shader_bin *gs_bin =
> > > - pipeline->shaders[MESA_SHADER_GEOMETRY];
> > > -
> > > - anv_batch_emit(&pipeline->batch, GENX(3DSTATE_GS), gs) {
> > > - gs.KernelStartPointer = gs_bin->kernel.offset;
> > > -
> > > - gs.ScratchSpaceBasePointer = (struct anv_address) {
> > > - .bo = anv_scratch_pool_alloc(device, &device-
> > > >scratch_pool,
> > > - MESA_SHADER_GEOMETRY,
> > > - gs_prog_data-
> > > >base.base.total_scratch),
> > > - .offset = 0,
> > > - };
> > > - gs.PerThreadScratchSpace =
> > > scratch_space(&gs_prog_data->base.base);
> > > -
> > > - gs.OutputVertexSize = gs_prog_data-
> > > >output_vertex_size_hwords * 2 - 1;
> > > - gs.OutputTopology = gs_prog_data-
> > > >output_topology;
> > > - gs.VertexURBEntryReadLength = gs_prog_data-
> > > >base.urb_read_length;
> > > - gs.IncludeVertexHandles = gs_prog_data-
> > > >base.include_vue_handles;
> > > -
> > > - gs.DispatchGRFStartRegisterForURBData =
> > > - gs_prog_data->base.base.dispatch_grf_start_reg;
> > > -
> > > - gs.SamplerCount =
> > get_sampler_count(gs_bin);
> > > - gs.BindingTableEntryCount =
> > > get_binding_table_entry_count(gs_bin);
> > > -
> > > - gs.MaximumNumberofThreads = devinfo->max_gs_threads
> > -
> > > 1;
> > > - /* This in the next dword on HSW. */
> > > - gs.ControlDataFormat = gs_prog_data-
> > > >control_data_format;
> > > - gs.ControlDataHeaderSize = gs_prog_data-
> > > >control_data_header_size_hwords;
> > > - gs.InstanceControl = MAX2(gs_prog_data-
> > > >invocations, 1) - 1;
> > > - gs.DispatchMode = gs_prog_data-
> > > >base.dispatch_mode;
> > > - gs.StatisticsEnable = true;
> > > - gs.IncludePrimitiveID = gs_prog_data-
> > > >include_primitive_id;
> > > -# if (GEN_IS_HASWELL)
> > > - gs.ReorderMode = REORDER_TRAILING;
> >
> > Shouldn't we have changed REORDER_TRAILING to TRAILING in
> > src/intel/genxml/gen75.xml in the previous patch?
>
> Yeah, I'll do that.
>
> > > -# else
> > > - gs.ReorderEnable = true;
> > > -# endif
> > > - gs.FunctionEnable = true;
> > > - }
> > > - }
> > > + emit_3dstate_gs(pipeline);
> > >
> > > if (!anv_pipeline_has_stage(pipeline, MESA_SHADER_FRAGMENT))
> > {
> > > anv_batch_emit(&pipeline->batch, GENX(3DSTATE_SBE), sbe);
> > > diff --git a/src/intel/vulkan/gen8_pipeline.c
> > > b/src/intel/vulkan/gen8_pipeline.c
> > > index 1320a13..5816bd4 100644
> > > --- a/src/intel/vulkan/gen8_pipeline.c
> > > +++ b/src/intel/vulkan/gen8_pipeline.c
> > > @@ -53,9 +53,6 @@ genX(graphics_pipeline_create)(
> > > {
> > > ANV_FROM_HANDLE(anv_device, device, _device);
> > > ANV_FROM_HANDLE(anv_render_pass, pass, pCreateInfo-
> > >renderPass);
> > > - const struct anv_physical_device *physical_device =
> > > - &device->instance->physicalDevice;
> > > - const struct gen_device_info *devinfo = &physical_device-
> > >info;
> > > struct anv_subpass *subpass = &pass->subpasses[pCreateInfo-
> > > >subpass];
> > > struct anv_pipeline *pipeline;
> > > VkResult result;
> > > @@ -112,64 +109,7 @@ genX(graphics_pipeline_create)(
> > > wm_prog_data ? wm_prog_data->barycentric_interp_modes :
> > 0;
> > > }
> > >
> > > - if (!anv_pipeline_has_stage(pipeline, MESA_SHADER_GEOMETRY))
> > {
> > > - anv_batch_emit(&pipeline->batch, GENX(3DSTATE_GS), gs);
> > > - } else {
> > > - const struct brw_gs_prog_data *gs_prog_data =
> > > get_gs_prog_data(pipeline);
> > > - const struct anv_shader_bin *gs_bin =
> > > - pipeline->shaders[MESA_SHADER_GEOMETRY];
> > > -
> > > - uint32_t offset = 1;
> > > - uint32_t length = (gs_prog_data->base.vue_map.num_slots +
> > 1) /
> > > 2 - offset;
> > > -
> > > - anv_batch_emit(&pipeline->batch, GENX(3DSTATE_GS), gs) {
> > > - gs.SingleProgramFlow = false;
> > > - gs.KernelStartPointer = gs_bin->kernel.offset;
> > > - gs.VectorMaskEnable = false;
> > > - gs.SamplerCount = get_sampler_count(gs_bin);
> > > - gs.BindingTableEntryCount =
> > > get_binding_table_entry_count(gs_bin);
> > > - gs.ExpectedVertexCount = gs_prog_data->vertices_in;
> > > -
> > > - gs.ScratchSpaceBasePointer = (struct anv_address) {
> > > - .bo = anv_scratch_pool_alloc(device, &device-
> > > >scratch_pool,
> > > - MESA_SHADER_GEOMETRY,
> > > - gs_prog_data-
> > > >base.base.total_scratch),
> > > - .offset = 0,
> > > - };
> > > - gs.PerThreadScratchSpace =
> > scratch_space(&gs_prog_data-
> > > >base.base);
> > > - gs.OutputVertexSize = gs_prog_data-
> > > >output_vertex_size_hwords * 2 - 1;
> > > - gs.OutputTopology = gs_prog_data-
> > >output_topology;
> > > - gs.VertexURBEntryReadLength = gs_prog_data-
> > > >base.urb_read_length;
> > > - gs.IncludeVertexHandles = gs_prog_data-
> > > >base.include_vue_handles;
> > > -
> > > - gs.DispatchGRFStartRegisterForURBData =
> > > - gs_prog_data->base.base.dispatch_grf_start_reg;
> > > -
> > > - gs.MaximumNumberofThreads = devinfo->max_gs_threads /
> > 2 -
> > > 1;
> > > - gs.ControlDataHeaderSize = gs_prog_data-
> > > >control_data_header_size_hwords;
> > > - gs.DispatchMode = gs_prog_data-
> > > >base.dispatch_mode;
> > > - gs.StatisticsEnable = true;
> > > - gs.IncludePrimitiveID = gs_prog_data-
> > > >include_primitive_id;
> > > - gs.ReorderMode = TRAILING;
> > > - gs.FunctionEnable = true;
> > > -
> > > - gs.ControlDataFormat = gs_prog_data-
> > > >control_data_format;
> > > -
> > > - gs.StaticOutput = gs_prog_data-
> > > >static_vertex_count >= 0;
> > > - gs.StaticOutputVertexCount =
> > > - gs_prog_data->static_vertex_count >= 0 ?
> > > - gs_prog_data->static_vertex_count : 0;
> > > -
> > > - /* FIXME: mesa sets this based on ctx-
> > > >Transform.ClipPlanesEnabled:
> > > - * UserClipDistanceClipTestEnableBitmask_3DSTATE_GS(v)
> > > - * UserClipDistanceCullTestEnableBitmask(v)
> > > - */
> > > -
> > > - gs.VertexURBEntryOutputReadOffset = offset;
> > > - gs.VertexURBEntryOutputLength = length;
> > > - }
> > > - }
> > > -
> > > + emit_3dstate_gs(pipeline);
> > > emit_3dstate_vs(pipeline);
> > >
> > > const int num_thread_bias = GEN_GEN == 8 ? 2 : 1;
> > > diff --git a/src/intel/vulkan/genX_pipeline_util.h
> > > b/src/intel/vulkan/genX_pipeline_util.h
> > > index 4fa96c8..515cc9a 100644
> > > --- a/src/intel/vulkan/genX_pipeline_util.h
> > > +++ b/src/intel/vulkan/genX_pipeline_util.h
> > > @@ -1053,4 +1053,77 @@ emit_3dstate_vs(struct anv_pipeline
> > *pipeline)
> > > }
> > > }
> > >
> > > +static void
> > > +emit_3dstate_gs(struct anv_pipeline *pipeline)
> > > +{
> > > + const struct gen_device_info *devinfo = &pipeline->device-
> > >info;
> > > + const struct anv_shader_bin *gs_bin =
> > > + pipeline->shaders[MESA_SHADER_GEOMETRY];
> > > +
> > > + if (!anv_pipeline_has_stage(pipeline, MESA_SHADER_GEOMETRY))
> > {
> > > + anv_batch_emit(&pipeline->batch, GENX(3DSTATE_GS), gs);
> > > + return;
> > > + }
> > > +
> > > + const struct brw_gs_prog_data *gs_prog_data =
> > > get_gs_prog_data(pipeline);
> > > +
> > > + anv_batch_emit(&pipeline->batch, GENX(3DSTATE_GS), gs) {
> > > + gs.FunctionEnable = true;
> > > + gs.StatisticsEnable = true;
> > > + gs.KernelStartPointer = gs_bin->kernel.offset;
> > > + gs.DispatchMode = gs_prog_data-
> > >base.dispatch_mode;
> > > +
> > > + gs.SingleProgramFlow = false;
> > > + gs.VectorMaskEnable = false;
> > > + gs.SamplerCount = get_sampler_count(gs_bin);
> > > + gs.BindingTableEntryCount =
> > > get_binding_table_entry_count(gs_bin);
> > > + gs.IncludeVertexHandles = gs_prog_data-
> > > >base.include_vue_handles;
> > > + gs.IncludePrimitiveID = gs_prog_data-
> > > >include_primitive_id;
> > > +
> > > + if (GEN_GEN == 8) {
> >
> > Just checking this doesn't need to be GEN_GEN >= 8 ??
> >
>
> No, "== 8" is correct. They upped the number of GS threads on gen8
> but the field was too small so they made you divide by 2. On Sky
> Lake the increased the size of the field and removed the divide by
> 2. Yeah, it's weird...
>
> > > + /* Broadwell is weird. It needs us to divide by 2. */
> > > + gs.MaximumNumberofThreads = devinfo->max_gs_threads / 2
> > -
> > > 1;
> > > + } else {
> > > + gs.MaximumNumberofThreads = devinfo->max_gs_threads -
> > 1;
> > > + }
> > > +
> > > + gs.OutputVertexSize = gs_prog_data-
> > > >output_vertex_size_hwords * 2 - 1
> > > + gs.OutputTopology = gs_prog_data-
> > >output_topology;
> > > + gs.VertexURBEntryReadLength = gs_prog_data-
> > > >base.urb_read_length;
> > > + gs.ControlDataFormat = gs_prog_data-
> > > >control_data_format;
> > > + gs.ControlDataHeaderSize = gs_prog_data-
> > > >control_data_header_size_hwords;
> > > + gs.InstanceControl = MAX2(gs_prog_data-
> > >invocations,
> > > 1) - 1;
> >
> > This wasn't previously set for gen8 I take it this doesn't matter?
> >
>
> I'll break that into its own patch.
>
> > > +#if GEN_GEN >= 8 || GEN_IS_HASWELL
> > > + gs.ReorderMode = TRAILING;
> > > +#else
> > > + gs.ReorderEnable = true;
> > > +#endif
> >
> > Maybe push this block over 1 more space so everything is aligned
> > together?
>
> What's not aligned about it? Do you want a blank line above it or
> something?
Sorry I should have put the comment under the line:
gs.VertexURBEntryReadLength = gs_prog_data->base.urb_read_length;
where the = is not aligned with the other lines. Since we are bothering
to try align all this stuff now would be a good time to make sure they
all line up.
>
> > > +
> > > +#if GEN_GEN >= 8
> > > + gs.ExpectedVertexCount = gs_prog_data->vertices_in;
> > > + gs.StaticOutput = gs_prog_data-
> > >static_vertex_count
> > > >= 0;
> > > + gs.StaticOutputVertexCount = gs_prog_data-
> > >static_vertex_count
> > > >= 0 ?
> > > + gs_prog_data-
> > >static_vertex_count
> > > : 0;
> > > +#endif
> > > +
> > > + gs.VertexURBEntryReadOffset = 0;
> > > + gs.VertexURBEntryReadLength = gs_prog_data-
> > > >base.urb_read_length;
> > > + gs.DispatchGRFStartRegisterForURBData =
> > > + gs_prog_data->base.base.dispatch_grf_start_reg;
> > > +
> > > +#if GEN_GEN >= 8
> > > + gs.VertexURBEntryOutputReadOffset =
> > get_urb_output_offset();
> > > + gs.VertexURBEntryOutputLength =
> > > get_urb_output_length(gs_bin);
> > > +
> > > + /* TODO */
> > > + gs.UserClipDistanceClipTestEnableBitmask = 0;
> > > + gs.UserClipDistanceCullTestEnableBitmask = 0;
> > > +#endif
> > > +
> > > + gs.PerThreadScratchSpace = get_scratch_space(gs_bin);
> > > + gs.ScratchSpaceBasePointer =
> > > + get_scratch_address(pipeline, MESA_SHADER_GEOMETRY,
> > > gs_bin);
> > > + }
> > > +}
> > > +
> > > #endif /* GENX_PIPELINE_UTIL_H */
> >
>
> _______________________________________________
> 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