[Mesa-dev] [PATCH 2/2] i965/genxml: Mostly style fixes for emit_vertices code.
Pohjolainen, Topi
topi.pohjolainen at gmail.com
Sat May 6 06:52:51 UTC 2017
On Fri, May 05, 2017 at 11:22:19AM -0700, Rafael Antognolli wrote:
> Several issues were caught on review after the original patch landed.
> This commit fixes them.
>
> Signed-off-by: Rafael Antognolli <rafael.antognolli at intel.com>
> Cc: "Pohjolainen, Topi" <topi.pohjolainen at gmail.com>
> ---
> src/mesa/drivers/dri/i965/genX_state_upload.c | 49 +++++++++++----------------
> 1 file changed, 20 insertions(+), 29 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index b8390ce..66b3264 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -360,7 +360,7 @@ is_passthru_format(uint32_t format)
> }
>
> UNUSED static int
> -genX(uploads_needed)(uint32_t format)
> +uploads_needed(uint32_t format)
> {
> if (!is_passthru_format(format))
> return 1;
> @@ -442,8 +442,8 @@ genX(emit_vertices)(struct brw_context *brw)
>
> #if GEN_GEN >= 8
> struct gl_context *ctx = &brw->ctx;
> - bool uses_edge_flag = (ctx->Polygon.FrontMode != GL_FILL ||
> - ctx->Polygon.BackMode != GL_FILL);
> + const bool uses_edge_flag = (ctx->Polygon.FrontMode != GL_FILL ||
> + ctx->Polygon.BackMode != GL_FILL);
>
> if (vs_prog_data->uses_vertexid || vs_prog_data->uses_instanceid) {
> unsigned vue = brw->vb.nr_enabled;
> @@ -512,7 +512,7 @@ genX(emit_vertices)(struct brw_context *brw)
> struct brw_vertex_element *input = brw->vb.enabled[i];
> uint32_t format = brw_get_vertex_surface_type(brw, input->glarray);
>
> - if (genX(uploads_needed(format)) > 1)
> + if (uploads_needed(format) > 1)
> nr_elements++;
> }
> #endif
> @@ -525,7 +525,8 @@ genX(emit_vertices)(struct brw_context *brw)
> * a VE loads from them.
> */
> if (nr_elements == 0) {
> - dw = brw_batch_emitn(brw, GENX(3DSTATE_VERTEX_ELEMENTS), 1 + GENX(VERTEX_ELEMENT_STATE_length));
> + dw = brw_batch_emitn(brw, GENX(3DSTATE_VERTEX_ELEMENTS),
> + 1 + GENX(VERTEX_ELEMENT_STATE_length));
> struct GENX(VERTEX_ELEMENT_STATE) elem = {
> .Valid = true,
> .SourceElementFormat = ISL_FORMAT_R32G32B32A32_FLOAT,
> @@ -546,11 +547,6 @@ genX(emit_vertices)(struct brw_context *brw)
> uses_draw_params + vs_prog_data->uses_drawid;
>
> if (nr_buffers) {
> -#if GEN_GEN >= 6
> - assert(nr_buffers <= 33);
> -#else
> - assert(nr_buffers <= 17);
> -#endif
> assert(nr_buffers <= (GEN_GEN >= 6 ? 33 : 17));
>
> dw = brw_batch_emitn(brw, GENX(3DSTATE_VERTEX_BUFFERS),
> @@ -563,11 +559,12 @@ genX(emit_vertices)(struct brw_context *brw)
> * half-float and 8 and 16-bit integer formats. This means that the
> * vertex element may poke over the end of the buffer by 2 bytes.
> */
> - unsigned padding =
> + const unsigned padding =
> (GEN_GEN <= 7 && !brw->is_baytrail && !brw->is_haswell) * 2;
> + const unsigned end = buffer->offset + buffer->size + padding;
> dw = genX(emit_vertex_buffer_state)(brw, dw, i, buffer->bo,
> buffer->offset,
> - buffer->offset + buffer->size + padding,
> + end + padding,
You included "padding" already in "end", you can drop it here.
> buffer->stride,
> buffer->step_rate);
> }
> @@ -596,22 +593,21 @@ genX(emit_vertices)(struct brw_context *brw)
> */
> #if GEN_GEN >= 6
> assert(nr_elements <= 34);
> - struct brw_vertex_element *gen6_edgeflag_input = NULL;
> + const struct brw_vertex_element *gen6_edgeflag_input = NULL;
> #else
> assert(nr_elements <= 18);
> #endif
>
> dw = brw_batch_emitn(brw, GENX(3DSTATE_VERTEX_ELEMENTS),
> 1 + GENX(VERTEX_ELEMENT_STATE_length) * nr_elements);
> - unsigned i;
> - for (i = 0; i < brw->vb.nr_enabled; i++) {
> - struct brw_vertex_element *input = brw->vb.enabled[i];
> + for (unsigned i = 0; i < brw->vb.nr_enabled; i++) {
> + const struct brw_vertex_element *input = brw->vb.enabled[i];
> uint32_t format = brw_get_vertex_surface_type(brw, input->glarray);
> uint32_t comp0 = VFCOMP_STORE_SRC;
> uint32_t comp1 = VFCOMP_STORE_SRC;
> uint32_t comp2 = VFCOMP_STORE_SRC;
> uint32_t comp3 = VFCOMP_STORE_SRC;
> - unsigned num_uploads = 1;
> + const unsigned num_uploads = GEN_GEN < 8 ? uploads_needed(format) : 1;
>
> #if GEN_GEN >= 8
> /* From the BDW PRM, Volume 2d, page 588 (VERTEX_ELEMENT_STATE):
> @@ -638,21 +634,16 @@ genX(emit_vertices)(struct brw_context *brw)
> }
> #endif
>
> -#if GEN_GEN < 8
> - num_uploads = genX(uploads_needed(format));
> -#endif
> -
> for (unsigned c = 0; c < num_uploads; c++) {
> - uint32_t upload_format = GEN_GEN >= 8 ? format :
> + const uint32_t upload_format = GEN_GEN >= 8 ? format :
> downsize_format_if_needed(format, c);
> /* If we need more that one upload, the offset stride would be 128
> * bits (16 bytes), as for previous uploads we are using the full
> * entry. */
> - unsigned int offset = input->offset + c * 16;
> - int size = input->glarray->Size;
> + const unsigned offset = input->offset + c * 16;
>
> - if (GEN_GEN < 8 && is_passthru_format(format))
> - size = upload_format_size(upload_format);
> + const int size = (GEN_GEN < 8 && is_passthru_format(format)) ?
> + upload_format_size(upload_format) : input->glarray->Size;
>
> switch (size) {
> case 0: comp0 = VFCOMP_STORE_0;
> @@ -722,7 +713,7 @@ genX(emit_vertices)(struct brw_context *brw)
> .Component2Control = VFCOMP_STORE_0,
> .Component3Control = VFCOMP_STORE_0,
> #if GEN_GEN < 5
> - .DestinationElementOffset = i * 4,
> + .DestinationElementOffset = brw->vb.nr_enabled * 4,
Given that we don't fully understand why both this and the one below use
the same offset (I get the feeling we don't have any test that enables them
both the same time), I'd drop these two changes afterall. I'd rather leave it
equivalent to what it was before your original change. Just to make it really
clear that you just changed emission mechanism but didn't make any functional
changes. That way any further fix/clarification is based on the original logic.
Thanks for the quick follow-up and sorry for the churn. Dropping these two
element offset changes and the extra padding further up:
Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> #endif
> };
>
> @@ -764,7 +755,7 @@ genX(emit_vertices)(struct brw_context *brw)
> .Component2Control = VFCOMP_STORE_0,
> .Component3Control = VFCOMP_STORE_0,
> #if GEN_GEN < 5
> - .DestinationElementOffset = i * 4,
> + .DestinationElementOffset = brw->vb.nr_enabled * 4,
> #endif
> };
>
> @@ -774,7 +765,7 @@ genX(emit_vertices)(struct brw_context *brw)
>
> #if GEN_GEN >= 6
> if (gen6_edgeflag_input) {
> - uint32_t format =
> + const uint32_t format =
> brw_get_vertex_surface_type(brw, gen6_edgeflag_input->glarray);
>
> struct GENX(VERTEX_ELEMENT_STATE) elem_state = {
> --
> 2.9.3
>
More information about the mesa-dev
mailing list