[Mesa-dev] [PATCH 11/19] r600g: atomize stencil ref state

Vadim Girlin vadimgirlin at gmail.com
Sun Sep 16 11:35:27 PDT 2012


On Sun, 2012-09-16 at 15:21 +0200, Marek Olšák wrote:
> You may try to move the call to r600_init_atom in
> evergreen_init_state_functions before the other atoms or experiment
> what positions works you.

I've tried to reorder the atoms but it doesn't help. Also I've noticed
that lockup only happens when both cpu and gpu power states are set to
high, so it looks like a race somewhere. I've tried bisect on kernel
(there was no lockup with v3.3), it resulted in the commit 285484e2
"drm/radeon: add support for evergreen/ni tiling informations v11". I
guess that patch simply affects the structure of the command stream by
enabling the tiling and uncovers real problem, same as with the
stencil_ref patch in mesa. 

So far I suspect it's more a kernel problem than a real gpu lockup,
maybe something with fences.

Vadim

> 
> Marek
> 
> On Sun, Sep 16, 2012 at 3:12 AM, Vadim Girlin <vadimgirlin at gmail.com> wrote:
> > Hi Marek,
> >
> > It seems after this patch something goes wrong for me on juniper - I've
> > got lockups with longprim test (when running with '-auto'). I don't see
> > anything wrong in the patch itself, so I guess it's order of writes or
> > something like that, maybe even not directly related to this patch but
> > uncovered by the changes in the command stream. Any ideas?
> >
> > Vadim
> >
> > On Tue, 2012-09-11 at 01:16 +0200, Marek Olšák wrote:
> >> ---
> >>  src/gallium/drivers/r600/evergreen_hw_context.c |    4 --
> >>  src/gallium/drivers/r600/evergreen_state.c      |    1 +
> >>  src/gallium/drivers/r600/r600_blit.c            |    4 +-
> >>  src/gallium/drivers/r600/r600_hw_context.c      |    7 ++--
> >>  src/gallium/drivers/r600/r600_pipe.h            |   45 ++++++++++++++---------
> >>  src/gallium/drivers/r600/r600_state.c           |    1 +
> >>  src/gallium/drivers/r600/r600_state_common.c    |   45 +++++++++++------------
> >>  7 files changed, 56 insertions(+), 51 deletions(-)
> >>
> >> diff --git a/src/gallium/drivers/r600/evergreen_hw_context.c b/src/gallium/drivers/r600/evergreen_hw_context.c
> >> index b2ea7e2..ff6c98d 100644
> >> --- a/src/gallium/drivers/r600/evergreen_hw_context.c
> >> +++ b/src/gallium/drivers/r600/evergreen_hw_context.c
> >> @@ -77,8 +77,6 @@ static const struct r600_reg evergreen_context_reg_list[] = {
> >>       {R_028418_CB_BLEND_GREEN, 0, 0},
> >>       {R_02841C_CB_BLEND_BLUE, 0, 0},
> >>       {R_028420_CB_BLEND_ALPHA, 0, 0},
> >> -     {R_028430_DB_STENCILREFMASK, 0, 0},
> >> -     {R_028434_DB_STENCILREFMASK_BF, 0, 0},
> >>       {R_02843C_PA_CL_VPORT_XSCALE_0, 0, 0},
> >>       {R_028440_PA_CL_VPORT_XOFFSET_0, 0, 0},
> >>       {R_028444_PA_CL_VPORT_YSCALE_0, 0, 0},
> >> @@ -408,8 +406,6 @@ static const struct r600_reg cayman_context_reg_list[] = {
> >>       {R_028418_CB_BLEND_GREEN, 0, 0},
> >>       {R_02841C_CB_BLEND_BLUE, 0, 0},
> >>       {R_028420_CB_BLEND_ALPHA, 0, 0},
> >> -     {R_028430_DB_STENCILREFMASK, 0, 0},
> >> -     {R_028434_DB_STENCILREFMASK_BF, 0, 0},
> >>       {R_02843C_PA_CL_VPORT_XSCALE_0, 0, 0},
> >>       {R_028440_PA_CL_VPORT_XOFFSET_0, 0, 0},
> >>       {R_028444_PA_CL_VPORT_YSCALE_0, 0, 0},
> >> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
> >> index bec9376..0bf517d 100644
> >> --- a/src/gallium/drivers/r600/evergreen_state.c
> >> +++ b/src/gallium/drivers/r600/evergreen_state.c
> >> @@ -2190,6 +2190,7 @@ void evergreen_init_state_functions(struct r600_context *rctx)
> >>       r600_init_atom(rctx, &rctx->cb_misc_state.atom, id++, evergreen_emit_cb_misc_state, 0);
> >>       r600_init_atom(rctx, &rctx->alphatest_state.atom, id++, r600_emit_alphatest_state, 6);
> >>       r600_init_atom(rctx, &rctx->db_misc_state.atom, id++, evergreen_emit_db_misc_state, 7);
> >> +     r600_init_atom(rctx, &rctx->stencil_ref.atom, id++, r600_emit_stencil_ref, 4);
> >>
> >>       rctx->context.create_blend_state = evergreen_create_blend_state;
> >>       rctx->context.create_depth_stencil_alpha_state = evergreen_create_dsa_state;
> >> diff --git a/src/gallium/drivers/r600/r600_blit.c b/src/gallium/drivers/r600/r600_blit.c
> >> index a49b453..d55f358 100644
> >> --- a/src/gallium/drivers/r600/r600_blit.c
> >> +++ b/src/gallium/drivers/r600/r600_blit.c
> >> @@ -68,9 +68,7 @@ static void r600_blitter_begin(struct pipe_context *ctx, enum r600_blitter_op op
> >>               util_blitter_save_fragment_shader(rctx->blitter, rctx->ps_shader);
> >>               util_blitter_save_blend(rctx->blitter, rctx->states[R600_PIPE_STATE_BLEND]);
> >>               util_blitter_save_depth_stencil_alpha(rctx->blitter, rctx->states[R600_PIPE_STATE_DSA]);
> >> -             if (rctx->states[R600_PIPE_STATE_STENCIL_REF]) {
> >> -                     util_blitter_save_stencil_ref(rctx->blitter, &rctx->stencil_ref);
> >> -             }
> >> +             util_blitter_save_stencil_ref(rctx->blitter, &rctx->stencil_ref.pipe_state);
> >>                  util_blitter_save_sample_mask(rctx->blitter, rctx->sample_mask.sample_mask);
> >>       }
> >>
> >> diff --git a/src/gallium/drivers/r600/r600_hw_context.c b/src/gallium/drivers/r600/r600_hw_context.c
> >> index 20feb7a..af03916 100644
> >> --- a/src/gallium/drivers/r600/r600_hw_context.c
> >> +++ b/src/gallium/drivers/r600/r600_hw_context.c
> >> @@ -335,8 +335,6 @@ static const struct r600_reg r600_context_reg_list[] = {
> >>       {R_028424_CB_FOG_RED, 0, 0},
> >>       {R_028428_CB_FOG_GREEN, 0, 0},
> >>       {R_02842C_CB_FOG_BLUE, 0, 0},
> >> -     {R_028430_DB_STENCILREFMASK, 0, 0},
> >> -     {R_028434_DB_STENCILREFMASK_BF, 0, 0},
> >>       {R_028780_CB_BLEND0_CONTROL, REG_FLAG_NOT_R600, 0},
> >>       {R_028784_CB_BLEND1_CONTROL, REG_FLAG_NOT_R600, 0},
> >>       {R_028788_CB_BLEND2_CONTROL, REG_FLAG_NOT_R600, 0},
> >> @@ -627,7 +625,7 @@ void r600_need_cs_space(struct r600_context *ctx, unsigned num_dw,
> >>               unsigned i;
> >>
> >>               /* The number of dwords all the dirty states would take. */
> >> -             for (i = 0; i < R600_MAX_ATOM; i++) {
> >> +             for (i = 0; i < R600_NUM_ATOMS; i++) {
> >>                       if (ctx->atoms[i] && ctx->atoms[i]->dirty) {
> >>                               num_dw += ctx->atoms[i]->num_dw;
> >>                       }
> >> @@ -1050,11 +1048,12 @@ void r600_begin_new_cs(struct r600_context *ctx)
> >>       r600_atom_dirty(ctx, &ctx->alphatest_state.atom);
> >>       r600_atom_dirty(ctx, &ctx->cb_misc_state.atom);
> >>       r600_atom_dirty(ctx, &ctx->db_misc_state.atom);
> >> +     r600_atom_dirty(ctx, &ctx->sample_mask.atom);
> >> +     r600_atom_dirty(ctx, &ctx->stencil_ref.atom);
> >>
> >>       if (ctx->chip_class <= R700) {
> >>               r600_atom_dirty(ctx, &ctx->seamless_cube_map.atom);
> >>       }
> >> -     r600_atom_dirty(ctx, &ctx->sample_mask.atom);
> >>
> >>       ctx->vertex_buffer_state.dirty_mask = ctx->vertex_buffer_state.enabled_mask;
> >>       r600_vertex_buffers_dirty(ctx);
> >> diff --git a/src/gallium/drivers/r600/r600_pipe.h b/src/gallium/drivers/r600/r600_pipe.h
> >> index 59fc592..ca25467 100644
> >> --- a/src/gallium/drivers/r600/r600_pipe.h
> >> +++ b/src/gallium/drivers/r600/r600_pipe.h
> >> @@ -35,7 +35,7 @@
> >>  #include "r600_resource.h"
> >>  #include "evergreen_compute.h"
> >>
> >> -#define R600_MAX_ATOM                                                20
> >> +#define R600_NUM_ATOMS 21
> >>
> >>  #define R600_MAX_CONST_BUFFERS 2
> >>  #define R600_MAX_CONST_BUFFER_SIZE 4096
> >> @@ -102,6 +102,19 @@ struct r600_sample_mask {
> >>       uint16_t sample_mask; /* there are only 8 bits on EG, 16 bits on Cayman */
> >>  };
> >>
> >> +struct r600_stencil_ref
> >> +{
> >> +     ubyte ref_value[2];
> >> +     ubyte valuemask[2];
> >> +     ubyte writemask[2];
> >> +};
> >> +
> >> +struct r600_stencil_ref_state {
> >> +     struct r600_atom atom;
> >> +     struct r600_stencil_ref state;
> >> +     struct pipe_stencil_ref pipe_state;
> >> +};
> >> +
> >>  enum r600_pipe_state_id {
> >>       R600_PIPE_STATE_BLEND = 0,
> >>       R600_PIPE_STATE_BLEND_COLOR,
> >> @@ -112,7 +125,6 @@ enum r600_pipe_state_id {
> >>       R600_PIPE_STATE_VGT,
> >>       R600_PIPE_STATE_FRAMEBUFFER,
> >>       R600_PIPE_STATE_DSA,
> >> -     R600_PIPE_STATE_STENCIL_REF,
> >>       R600_PIPE_STATE_POLYGON_OFFSET,
> >>       R600_PIPE_STATE_FETCH_SHADER,
> >>       R600_PIPE_NSTATES
> >> @@ -284,13 +296,6 @@ struct r600_fence_block {
> >>  #define R600_CONSTANT_ARRAY_SIZE 256
> >>  #define R600_RESOURCE_ARRAY_SIZE 160
> >>
> >> -struct r600_stencil_ref
> >> -{
> >> -     ubyte ref_value[2];
> >> -     ubyte valuemask[2];
> >> -     ubyte writemask[2];
> >> -};
> >> -
> >>  struct r600_constbuf_state
> >>  {
> >>       struct r600_atom                atom;
> >> @@ -329,7 +334,6 @@ struct r600_context {
> >>       unsigned                        pa_sc_line_stipple;
> >>       unsigned                        pa_cl_clip_cntl;
> >>       /* for saving when using blitter */
> >> -     struct pipe_stencil_ref         stencil_ref;
> >>       struct pipe_viewport_state      viewport;
> >>       struct pipe_clip_state          clip;
> >>       struct r600_pipe_shader_selector        *ps_shader;
> >> @@ -357,24 +361,30 @@ struct r600_context {
> >>
> >>       unsigned default_ps_gprs, default_vs_gprs;
> >>
> >> +     /******************************/
> >>       /* States based on r600_atom. */
> >> +     struct r600_atom                *atoms[R600_NUM_ATOMS];
> >> +     /* States for CS initialization. */
> >>       struct r600_command_buffer      start_cs_cmd; /* invariant state mostly */
> >> -     struct r600_atom                *atoms[R600_MAX_ATOM];
> >>       /** Compute specific registers initializations.  The start_cs_cmd atom
> >>        *  must be emitted before start_compute_cs_cmd. */
> >> -        struct r600_command_buffer      start_compute_cs_cmd;
> >> +     struct r600_command_buffer      start_compute_cs_cmd;
> >> +     /* Register states. */
> >>       struct r600_alphatest_state     alphatest_state;
> >>       struct r600_cb_misc_state       cb_misc_state;
> >>       struct r600_db_misc_state       db_misc_state;
> >> +     struct r600_seamless_cube_map   seamless_cube_map;
> >> +     struct r600_stencil_ref_state   stencil_ref;
> >> +     struct r600_sample_mask         sample_mask;
> >> +     /* Shaders and shader resources. */
> >> +     struct r600_cs_shader_state     cs_shader_state;
> >> +     struct r600_constbuf_state      constbuf_state[PIPE_SHADER_TYPES];
> >> +     struct r600_textures_info       samplers[PIPE_SHADER_TYPES];
> >>       /** Vertex buffers for fetch shaders */
> >>       struct r600_vertexbuf_state     vertex_buffer_state;
> >>       /** Vertex buffers for compute shaders */
> >>       struct r600_vertexbuf_state     cs_vertex_buffer_state;
> >> -     struct r600_constbuf_state      constbuf_state[PIPE_SHADER_TYPES];
> >> -     struct r600_textures_info       samplers[PIPE_SHADER_TYPES];
> >> -     struct r600_seamless_cube_map   seamless_cube_map;
> >> -     struct r600_cs_shader_state     cs_shader_state;
> >> -     struct r600_sample_mask         sample_mask;
> >> +     /******************************/
> >>
> >>       /* current external blend state (from state tracker) */
> >>       struct r600_pipe_blend          *blend;
> >> @@ -566,6 +576,7 @@ void r600_translate_index_buffer(struct r600_context *r600,
> >>  /* r600_state_common.c */
> >>  void r600_init_common_state_functions(struct r600_context *rctx);
> >>  void r600_emit_alphatest_state(struct r600_context *rctx, struct r600_atom *atom);
> >> +void r600_emit_stencil_ref(struct r600_context *rctx, struct r600_atom *atom);
> >>  void r600_init_atom(struct r600_context *rctx, struct r600_atom *atom, unsigned id,
> >>                   void (*emit)(struct r600_context *ctx, struct r600_atom *state),
> >>                   unsigned num_dw);
> >> diff --git a/src/gallium/drivers/r600/r600_state.c b/src/gallium/drivers/r600/r600_state.c
> >> index 1ba839c..eb0e6f4 100644
> >> --- a/src/gallium/drivers/r600/r600_state.c
> >> +++ b/src/gallium/drivers/r600/r600_state.c
> >> @@ -2071,6 +2071,7 @@ void r600_init_state_functions(struct r600_context *rctx)
> >>       r600_init_atom(rctx, &rctx->cb_misc_state.atom, id++, r600_emit_cb_misc_state, 0);
> >>       r600_init_atom(rctx, &rctx->alphatest_state.atom, id++, r600_emit_alphatest_state, 6);
> >>       r600_init_atom(rctx, &rctx->db_misc_state.atom, id++, r600_emit_db_misc_state, 4);
> >> +     r600_init_atom(rctx, &rctx->stencil_ref.atom, id++, r600_emit_stencil_ref, 4);
> >>
> >>       rctx->context.create_blend_state = r600_create_blend_state;
> >>       rctx->context.create_depth_stencil_alpha_state = r600_create_dsa_state;
> >> diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c
> >> index 7899e6c..b3c3c82 100644
> >> --- a/src/gallium/drivers/r600/r600_state_common.c
> >> +++ b/src/gallium/drivers/r600/r600_state_common.c
> >> @@ -62,7 +62,7 @@ void r600_init_atom(struct r600_context *rctx,
> >>                   void (*emit)(struct r600_context *ctx, struct r600_atom *state),
> >>                   unsigned num_dw)
> >>  {
> >> -     assert(id < R600_MAX_ATOM);
> >> +     assert(id < R600_NUM_ATOMS);
> >>       assert(rctx->atoms[id] == NULL);
> >>       rctx->atoms[id] = atom;
> >>       atom->id = id;
> >> @@ -197,26 +197,25 @@ static void r600_set_stencil_ref(struct pipe_context *ctx,
> >>                                const struct r600_stencil_ref *state)
> >>  {
> >>       struct r600_context *rctx = (struct r600_context *)ctx;
> >> -     struct r600_pipe_state *rstate = CALLOC_STRUCT(r600_pipe_state);
> >>
> >> -     if (rstate == NULL)
> >> -             return;
> >> +     rctx->stencil_ref.state = *state;
> >> +     r600_atom_dirty(rctx, &rctx->stencil_ref.atom);
> >> +}
> >>
> >> -     rstate->id = R600_PIPE_STATE_STENCIL_REF;
> >> -     r600_pipe_state_add_reg(rstate,
> >> -                             R_028430_DB_STENCILREFMASK,
> >> -                             S_028430_STENCILREF(state->ref_value[0]) |
> >> -                             S_028430_STENCILMASK(state->valuemask[0]) |
> >> -                             S_028430_STENCILWRITEMASK(state->writemask[0]));
> >> -     r600_pipe_state_add_reg(rstate,
> >> -                             R_028434_DB_STENCILREFMASK_BF,
> >> -                             S_028434_STENCILREF_BF(state->ref_value[1]) |
> >> -                             S_028434_STENCILMASK_BF(state->valuemask[1]) |
> >> -                             S_028434_STENCILWRITEMASK_BF(state->writemask[1]));
> >> -
> >> -     free(rctx->states[R600_PIPE_STATE_STENCIL_REF]);
> >> -     rctx->states[R600_PIPE_STATE_STENCIL_REF] = rstate;
> >> -     r600_context_pipe_state_set(rctx, rstate);
> >> +void r600_emit_stencil_ref(struct r600_context *rctx, struct r600_atom *atom)
> >> +{
> >> +     struct radeon_winsys_cs *cs = rctx->cs;
> >> +     struct r600_stencil_ref_state *a = (struct r600_stencil_ref_state*)atom;
> >> +
> >> +     r600_write_context_reg_seq(cs, R_028430_DB_STENCILREFMASK, 2);
> >> +     r600_write_value(cs, /* R_028430_DB_STENCILREFMASK */
> >> +                      S_028430_STENCILREF(a->state.ref_value[0]) |
> >> +                      S_028430_STENCILMASK(a->state.valuemask[0]) |
> >> +                      S_028430_STENCILWRITEMASK(a->state.writemask[0]));
> >> +     r600_write_value(cs, /* R_028434_DB_STENCILREFMASK_BF */
> >> +                      S_028434_STENCILREF_BF(a->state.ref_value[1]) |
> >> +                      S_028434_STENCILMASK_BF(a->state.valuemask[1]) |
> >> +                      S_028434_STENCILWRITEMASK_BF(a->state.writemask[1]));
> >>  }
> >>
> >>  static void r600_set_pipe_stencil_ref(struct pipe_context *ctx,
> >> @@ -226,7 +225,7 @@ static void r600_set_pipe_stencil_ref(struct pipe_context *ctx,
> >>       struct r600_pipe_dsa *dsa = (struct r600_pipe_dsa*)rctx->states[R600_PIPE_STATE_DSA];
> >>       struct r600_stencil_ref ref;
> >>
> >> -     rctx->stencil_ref = *state;
> >> +     rctx->stencil_ref.pipe_state = *state;
> >>
> >>       if (!dsa)
> >>               return;
> >> @@ -254,8 +253,8 @@ static void r600_bind_dsa_state(struct pipe_context *ctx, void *state)
> >>       rctx->states[rstate->id] = rstate;
> >>       r600_context_pipe_state_set(rctx, rstate);
> >>
> >> -     ref.ref_value[0] = rctx->stencil_ref.ref_value[0];
> >> -     ref.ref_value[1] = rctx->stencil_ref.ref_value[1];
> >> +     ref.ref_value[0] = rctx->stencil_ref.pipe_state.ref_value[0];
> >> +     ref.ref_value[1] = rctx->stencil_ref.pipe_state.ref_value[1];
> >>       ref.valuemask[0] = dsa->valuemask[0];
> >>       ref.valuemask[1] = dsa->valuemask[1];
> >>       ref.writemask[0] = dsa->writemask[0];
> >> @@ -1208,7 +1207,7 @@ static void r600_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info
> >>       r600_need_cs_space(rctx, 0, TRUE);
> >>       r600_flush_emit(rctx);
> >>
> >> -     for (i = 0; i < R600_MAX_ATOM; i++) {
> >> +     for (i = 0; i < R600_NUM_ATOMS; i++) {
> >>               if (rctx->atoms[i] == NULL || !rctx->atoms[i]->dirty) {
> >>                       continue;
> >>               }
> >
> >
> >





More information about the mesa-dev mailing list