From 383f4065911314906fb23e639308e68a5e40524d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Thu, 18 Apr 2019 15:43:46 -0400 Subject: [PATCH] radeonsi: remove dirty slot masks from scissor and viewport states All registers in the array need to be updated if any of them is changed. Only apps writing gl_ViewportIndex were affected by this bug. --- src/gallium/drivers/radeonsi/si_blit.c | 2 +- src/gallium/drivers/radeonsi/si_gfx_cs.c | 3 - src/gallium/drivers/radeonsi/si_pipe.h | 9 +- src/gallium/drivers/radeonsi/si_state.c | 8 +- src/gallium/drivers/radeonsi/si_state_draw.c | 10 +- .../drivers/radeonsi/si_state_viewport.c | 101 ++++++------------ 6 files changed, 40 insertions(+), 93 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_blit.c b/src/gallium/drivers/radeonsi/si_blit.c index 610de289a20..9d3d7d3d27a 100644 --- a/src/gallium/drivers/radeonsi/si_blit.c +++ b/src/gallium/drivers/radeonsi/si_blit.c @@ -58,7 +58,7 @@ void si_blitter_begin(struct si_context *sctx, enum si_blitter_op op) util_blitter_save_stencil_ref(sctx->blitter, &sctx->stencil_ref.state); util_blitter_save_fragment_shader(sctx->blitter, sctx->ps_shader.cso); util_blitter_save_sample_mask(sctx->blitter, sctx->sample_mask); - util_blitter_save_scissor(sctx->blitter, &sctx->scissors.states[0]); + util_blitter_save_scissor(sctx->blitter, &sctx->scissors[0]); util_blitter_save_window_rectangles(sctx->blitter, sctx->window_rectangles_include, sctx->num_window_rectangles, diff --git a/src/gallium/drivers/radeonsi/si_gfx_cs.c b/src/gallium/drivers/radeonsi/si_gfx_cs.c index 3e369234597..d0d405c473f 100644 --- a/src/gallium/drivers/radeonsi/si_gfx_cs.c +++ b/src/gallium/drivers/radeonsi/si_gfx_cs.c @@ -315,9 +315,6 @@ void si_begin_new_gfx_cs(struct si_context *ctx) if (!has_clear_state || ctx->num_window_rectangles > 0) si_mark_atom_dirty(ctx, &ctx->atoms.s.window_rectangles); - ctx->scissors.dirty_mask = (1 << SI_MAX_VIEWPORTS) - 1; - ctx->viewports.dirty_mask = (1 << SI_MAX_VIEWPORTS) - 1; - ctx->viewports.depth_range_dirty_mask = (1 << SI_MAX_VIEWPORTS) - 1; si_mark_atom_dirty(ctx, &ctx->atoms.s.guardband); si_mark_atom_dirty(ctx, &ctx->atoms.s.scissors); si_mark_atom_dirty(ctx, &ctx->atoms.s.viewports); diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h index 1d26ca90219..d3ddb912245 100644 --- a/src/gallium/drivers/radeonsi/si_pipe.h +++ b/src/gallium/drivers/radeonsi/si_pipe.h @@ -672,14 +672,7 @@ struct si_signed_scissor { enum si_quant_mode quant_mode; }; -struct si_scissors { - unsigned dirty_mask; - struct pipe_scissor_state states[SI_MAX_VIEWPORTS]; -}; - struct si_viewports { - unsigned dirty_mask; - unsigned depth_range_dirty_mask; struct pipe_viewport_state states[SI_MAX_VIEWPORTS]; struct si_signed_scissor as_scissor[SI_MAX_VIEWPORTS]; }; @@ -883,7 +876,7 @@ struct si_context { struct si_clip_state clip_state; struct si_shader_data shader_pointers; struct si_stencil_ref stencil_ref; - struct si_scissors scissors; + struct pipe_scissor_state scissors[SI_MAX_VIEWPORTS]; struct si_streamout streamout; struct si_viewports viewports; unsigned num_window_rectangles; diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c index bc7e777ad73..2266b0a0824 100644 --- a/src/gallium/drivers/radeonsi/si_state.c +++ b/src/gallium/drivers/radeonsi/si_state.c @@ -1015,10 +1015,8 @@ static void si_bind_rs_state(struct pipe_context *ctx, void *state) si_update_poly_offset_state(sctx); if (!old_rs || - old_rs->scissor_enable != rs->scissor_enable) { - sctx->scissors.dirty_mask = (1 << SI_MAX_VIEWPORTS) - 1; + old_rs->scissor_enable != rs->scissor_enable) si_mark_atom_dirty(sctx, &sctx->atoms.s.scissors); - } if (!old_rs || old_rs->line_width != rs->line_width || @@ -1027,10 +1025,8 @@ static void si_bind_rs_state(struct pipe_context *ctx, void *state) si_mark_atom_dirty(sctx, &sctx->atoms.s.guardband); if (!old_rs || - old_rs->clip_halfz != rs->clip_halfz) { - sctx->viewports.depth_range_dirty_mask = (1 << SI_MAX_VIEWPORTS) - 1; + old_rs->clip_halfz != rs->clip_halfz) si_mark_atom_dirty(sctx, &sctx->atoms.s.viewports); - } if (!old_rs || old_rs->clip_plane_enable != rs->clip_plane_enable || diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c b/src/gallium/drivers/radeonsi/si_state_draw.c index 4b60679484f..8e01e1b35e1 100644 --- a/src/gallium/drivers/radeonsi/si_state_draw.c +++ b/src/gallium/drivers/radeonsi/si_state_draw.c @@ -1487,10 +1487,9 @@ static void si_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info *i if (has_gfx9_scissor_bug && (sctx->context_roll || - si_is_atom_dirty(sctx, &sctx->atoms.s.scissors))) { - sctx->scissors.dirty_mask = (1 << SI_MAX_VIEWPORTS) - 1; + si_is_atom_dirty(sctx, &sctx->atoms.s.scissors))) sctx->atoms.s.scissors.emit(sctx); - } + sctx->dirty_atoms = 0; si_emit_draw_packets(sctx, info, indexbuf, index_size, index_offset); @@ -1519,10 +1518,9 @@ static void si_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info *i if (has_gfx9_scissor_bug && (sctx->context_roll || - si_is_atom_dirty(sctx, &sctx->atoms.s.scissors))) { - sctx->scissors.dirty_mask = (1 << SI_MAX_VIEWPORTS) - 1; + si_is_atom_dirty(sctx, &sctx->atoms.s.scissors))) sctx->atoms.s.scissors.emit(sctx); - } + sctx->dirty_atoms = 0; si_emit_draw_packets(sctx, info, indexbuf, index_size, index_offset); diff --git a/src/gallium/drivers/radeonsi/si_state_viewport.c b/src/gallium/drivers/radeonsi/si_state_viewport.c index 6f348a9b58d..792d1c4efd1 100644 --- a/src/gallium/drivers/radeonsi/si_state_viewport.c +++ b/src/gallium/drivers/radeonsi/si_state_viewport.c @@ -37,13 +37,12 @@ static void si_set_scissor_states(struct pipe_context *pctx, int i; for (i = 0; i < num_scissors; i++) - ctx->scissors.states[start_slot + i] = state[i]; + ctx->scissors[start_slot + i] = state[i]; if (!ctx->queued.named.rasterizer || !ctx->queued.named.rasterizer->scissor_enable) return; - ctx->scissors.dirty_mask |= ((1 << num_scissors) - 1) << start_slot; si_mark_atom_dirty(ctx, &ctx->atoms.s.scissors); } @@ -289,36 +288,27 @@ static void si_emit_guardband(struct si_context *ctx) static void si_emit_scissors(struct si_context *ctx) { struct radeon_cmdbuf *cs = ctx->gfx_cs; - struct pipe_scissor_state *states = ctx->scissors.states; - unsigned mask = ctx->scissors.dirty_mask; + struct pipe_scissor_state *states = ctx->scissors; bool scissor_enabled = ctx->queued.named.rasterizer->scissor_enable; /* The simple case: Only 1 viewport is active. */ if (!ctx->vs_writes_viewport_index) { struct si_signed_scissor *vp = &ctx->viewports.as_scissor[0]; - if (!(mask & 1)) - return; - radeon_set_context_reg_seq(cs, R_028250_PA_SC_VPORT_SCISSOR_0_TL, 2); si_emit_one_scissor(ctx, cs, vp, scissor_enabled ? &states[0] : NULL); - ctx->scissors.dirty_mask &= ~1; /* clear one bit */ return; } - while (mask) { - int start, count, i; - - u_bit_scan_consecutive_range(&mask, &start, &count); - - radeon_set_context_reg_seq(cs, R_028250_PA_SC_VPORT_SCISSOR_0_TL + - start * 4 * 2, count * 2); - for (i = start; i < start+count; i++) { - si_emit_one_scissor(ctx, cs, &ctx->viewports.as_scissor[i], - scissor_enabled ? &states[i] : NULL); - } + /* All registers in the array need to be updated if any of them is changed. + * This is a hardware requirement. + */ + radeon_set_context_reg_seq(cs, R_028250_PA_SC_VPORT_SCISSOR_0_TL, + SI_MAX_VIEWPORTS * 2); + for (unsigned i = 0; i < SI_MAX_VIEWPORTS; i++) { + si_emit_one_scissor(ctx, cs, &ctx->viewports.as_scissor[i], + scissor_enabled ? &states[i] : NULL); } - ctx->scissors.dirty_mask = 0; } static void si_set_viewport_states(struct pipe_context *pctx, @@ -327,7 +317,6 @@ static void si_set_viewport_states(struct pipe_context *pctx, const struct pipe_viewport_state *state) { struct si_context *ctx = (struct si_context *)pctx; - unsigned mask; int i; for (i = 0; i < num_viewports; i++) { @@ -392,10 +381,6 @@ static void si_set_viewport_states(struct pipe_context *pctx, scissor->quant_mode = SI_QUANT_MODE_16_8_FIXED_POINT_1_256TH; } - mask = ((1 << num_viewports) - 1) << start_slot; - ctx->viewports.dirty_mask |= mask; - ctx->viewports.depth_range_dirty_mask |= mask; - ctx->scissors.dirty_mask |= mask; si_mark_atom_dirty(ctx, &ctx->atoms.s.viewports); si_mark_atom_dirty(ctx, &ctx->atoms.s.guardband); si_mark_atom_dirty(ctx, &ctx->atoms.s.scissors); @@ -418,30 +403,21 @@ static void si_emit_viewports(struct si_context *ctx) { struct radeon_cmdbuf *cs = ctx->gfx_cs; struct pipe_viewport_state *states = ctx->viewports.states; - unsigned mask = ctx->viewports.dirty_mask; /* The simple case: Only 1 viewport is active. */ if (!ctx->vs_writes_viewport_index) { - if (!(mask & 1)) - return; - radeon_set_context_reg_seq(cs, R_02843C_PA_CL_VPORT_XSCALE, 6); si_emit_one_viewport(ctx, &states[0]); - ctx->viewports.dirty_mask &= ~1; /* clear one bit */ return; } - while (mask) { - int start, count, i; - - u_bit_scan_consecutive_range(&mask, &start, &count); - - radeon_set_context_reg_seq(cs, R_02843C_PA_CL_VPORT_XSCALE + - start * 4 * 6, count * 6); - for (i = start; i < start+count; i++) - si_emit_one_viewport(ctx, &states[i]); - } - ctx->viewports.dirty_mask = 0; + /* All registers in the array need to be updated if any of them is changed. + * This is a hardware requirement. + */ + radeon_set_context_reg_seq(cs, R_02843C_PA_CL_VPORT_XSCALE + + 0, SI_MAX_VIEWPORTS * 6); + for (unsigned i = 0; i < SI_MAX_VIEWPORTS; i++) + si_emit_one_viewport(ctx, &states[i]); } static inline void @@ -460,41 +436,32 @@ static void si_emit_depth_ranges(struct si_context *ctx) { struct radeon_cmdbuf *cs = ctx->gfx_cs; struct pipe_viewport_state *states = ctx->viewports.states; - unsigned mask = ctx->viewports.depth_range_dirty_mask; bool clip_halfz = ctx->queued.named.rasterizer->clip_halfz; bool window_space = ctx->vs_disables_clipping_viewport; float zmin, zmax; /* The simple case: Only 1 viewport is active. */ if (!ctx->vs_writes_viewport_index) { - if (!(mask & 1)) - return; - si_viewport_zmin_zmax(&states[0], clip_halfz, window_space, &zmin, &zmax); radeon_set_context_reg_seq(cs, R_0282D0_PA_SC_VPORT_ZMIN_0, 2); radeon_emit(cs, fui(zmin)); radeon_emit(cs, fui(zmax)); - ctx->viewports.depth_range_dirty_mask &= ~1; /* clear one bit */ return; } - while (mask) { - int start, count, i; - - u_bit_scan_consecutive_range(&mask, &start, &count); - - radeon_set_context_reg_seq(cs, R_0282D0_PA_SC_VPORT_ZMIN_0 + - start * 4 * 2, count * 2); - for (i = start; i < start+count; i++) { - si_viewport_zmin_zmax(&states[i], clip_halfz, window_space, - &zmin, &zmax); - radeon_emit(cs, fui(zmin)); - radeon_emit(cs, fui(zmax)); - } + /* All registers in the array need to be updated if any of them is changed. + * This is a hardware requirement. + */ + radeon_set_context_reg_seq(cs, R_0282D0_PA_SC_VPORT_ZMIN_0, + SI_MAX_VIEWPORTS * 2); + for (unsigned i = 0; i < SI_MAX_VIEWPORTS; i++) { + si_viewport_zmin_zmax(&states[i], clip_halfz, window_space, + &zmin, &zmax); + radeon_emit(cs, fui(zmin)); + radeon_emit(cs, fui(zmax)); } - ctx->viewports.depth_range_dirty_mask = 0; } static void si_emit_viewport_states(struct si_context *ctx) @@ -527,8 +494,6 @@ void si_update_vs_viewport_state(struct si_context *ctx) if (ctx->vs_disables_clipping_viewport != vs_window_space) { ctx->vs_disables_clipping_viewport = vs_window_space; - ctx->scissors.dirty_mask = (1 << SI_MAX_VIEWPORTS) - 1; - ctx->viewports.depth_range_dirty_mask = (1 << SI_MAX_VIEWPORTS) - 1; si_mark_atom_dirty(ctx, &ctx->atoms.s.scissors); si_mark_atom_dirty(ctx, &ctx->atoms.s.viewports); } @@ -541,15 +506,13 @@ void si_update_vs_viewport_state(struct si_context *ctx) ctx->vs_writes_viewport_index = info->writes_viewport_index; si_mark_atom_dirty(ctx, &ctx->atoms.s.guardband); - if (!ctx->vs_writes_viewport_index) - return; - - if (ctx->scissors.dirty_mask) + /* Emit scissors and viewports that were enabled by having + * the ViewportIndex output. + */ + if (info->writes_viewport_index) { si_mark_atom_dirty(ctx, &ctx->atoms.s.scissors); - - if (ctx->viewports.dirty_mask || - ctx->viewports.depth_range_dirty_mask) si_mark_atom_dirty(ctx, &ctx->atoms.s.viewports); + } } static void si_emit_window_rectangles(struct si_context *sctx) -- 2.30.2