[Mesa-dev] [PATCH 2/2] r600g: add htile support v14

[Marek Olšák]

The patch looks good mostly. There are some comments below.

> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
> index 58964c4..39de55b 100644
> --- a/src/gallium/drivers/r600/evergreen_state.c
> +++ b/src/gallium/drivers/r600/evergreen_state.c
[snip]
> @@ -2112,6 +2160,10 @@ static void evergreen_emit_db_misc_state(struct r600_context *rctx, struct r600_
>                                      S_028000_STENCIL_COMPRESS_DISABLE(1);
>                 db_render_override |= S_02800C_DISABLE_PIXEL_RATE_TILES(1);
>         }
> +       if (a->htile_clear) {
> +               /* FIXME we might want to disable cliprect here */

This comment is unnecessary, because we don't use clip rectangles.

> +               db_render_control |= S_028000_DEPTH_CLEAR_ENABLE(1);
> +       }
>
>         r600_write_context_reg_seq(cs, R_028000_DB_RENDER_CONTROL, 2);
>         r600_write_value(cs, db_render_control); /* R_028000_DB_RENDER_CONTROL */
> @@ -2424,6 +2476,7 @@ void evergreen_init_state_functions(struct r600_context *rctx)
>         r600_init_atom(rctx, &rctx->clip_misc_state.atom, id++, r600_emit_clip_misc_state, 6);
>         r600_init_atom(rctx, &rctx->clip_state.atom, id++, evergreen_emit_clip_state, 26);
>         r600_init_atom(rctx, &rctx->db_misc_state.atom, id++, evergreen_emit_db_misc_state, 10);
> +       r600_init_atom(rctx, &rctx->db_state.atom, id++, evergreen_emit_db_state, 15);

If I am counting correctly, db_state emits 14 dwords at most, not 15.

>         r600_init_atom(rctx, &rctx->dsa_state.atom, id++, r600_emit_cso_state, 0);
>         r600_init_atom(rctx, &rctx->poly_offset_state.atom, id++, evergreen_emit_polygon_offset, 6);
>         r600_init_atom(rctx, &rctx->rasterizer_state.atom, id++, r600_emit_cso_state, 0);
[snip]
> diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
> index 19147d9..4382751 100644
> --- a/src/gallium/drivers/r600/r600_pipe.c
> +++ b/src/gallium/drivers/r600/r600_pipe.c
> @@ -1032,6 +1032,16 @@ struct pipe_screen *r600_screen_create(struct radeon_winsys *ws)
>         LIST_INITHEAD(&rscreen->fences.blocks);
>         pipe_mutex_init(rscreen->fences.mutex);
>
> +       /* Hyperz leads to lockup on r6xx/r7xx and evergreen, due to this instabilities
> +        * don't enable this by default until we can figure out how to do it properly
> +        *
> +        * You can trigger lockup easily with :
> +        * piglit/bin/depthstencil-render-miplevels 1024 d=s=z24_s8
> +        * run it in a loop, it will lockup often on first run
> +        */

Is this comment still needed? It's not consistent with the code below
(enabling Hyper-Z by default).

> +       rscreen->use_hyperz = debug_get_bool_option("R600_HYPERZ", TRUE);
> +       rscreen->use_hyperz = rscreen->info.drm_minor >= 26 ? rscreen->use_hyperz : FALSE;
> +
>         rscreen->global_pool = compute_memory_pool_new(rscreen);
>
>         return &rscreen->screen;
[snip]
> diff --git a/src/gallium/drivers/r600/r600_resource.h b/src/gallium/drivers/r600/r600_resource.h
> index 007d5e0..dd0b613 100644
> --- a/src/gallium/drivers/r600/r600_resource.h
> +++ b/src/gallium/drivers/r600/r600_resource.h
> @@ -60,6 +60,10 @@ struct r600_texture {
>          * MSAA textures cannot have mipmaps. */
>         unsigned                        fmask_offset, fmask_size, fmask_bank_height;
>         unsigned                        cmask_offset, cmask_size, cmask_slice_tile_max;
> +
> +       struct r600_resource            *htile;
> +       /* use htile only for first level */
> +       float                           depth_clear;
>  };

Since this commit:

commit 39737e17e7a61535a35669756161005a7a5c887b
Author: Marek Olšák <maraeo at gmail.com>
Date:   Mon Dec 3 01:26:22 2012 +0100

    st/dri: always allocate private depth-stencil buffers

The zbuffer allocation has been moved to Mesa. That means:

1) The HTILE buffer will always be allocated. There's no need to
update the DDX, because the DDX doesn't allocate zbuffers anymore.
(and it needs no changes in r600g)

2) This is maybe more related to your patch: We can put the HTILE
buffer in the same resource as the zbuffer in the same way CMASK and
FMASK are allocated.

>
>  #define R600_TEX_IS_TILED(tex, level) ((tex)->array_mode[level] != V_038000_ARRAY_LINEAR_GENERAL && (tex)->array_mode[level] != V_038000_ARRAY_LINEAR_ALIGNED)
> @@ -113,6 +117,11 @@ struct r600_surface {
>         unsigned db_stencil_info;       /* EG only */
>         unsigned db_prefetch_limit;     /* R600 only */
>         unsigned pa_su_poly_offset_db_fmt_cntl;
> +
> +       unsigned                        htile_enabled;
> +       unsigned                        db_htile_surface;
> +       unsigned                        db_htile_data_base;
> +       unsigned                        db_preload_control;
>  };
>
>  /* Return if the depth format can be read without the DB->CB copy on r6xx-r7xx. */
[snip]
> diff --git a/src/gallium/drivers/r600/r600_texture.c b/src/gallium/drivers/r600/r600_texture.c
> index 56e9b64..dc23c5f 100644
> --- a/src/gallium/drivers/r600/r600_texture.c
> +++ b/src/gallium/drivers/r600/r600_texture.c
> @@ -438,6 +438,45 @@ r600_texture_create_object(struct pipe_screen *screen,
>         /* Tiled depth textures utilize the non-displayable tile order. */
>         rtex->non_disp_tiling = rtex->is_depth && rtex->surface.level[0].mode >= RADEON_SURF_MODE_1D;
>
> +       /* only enable hyperz for PIPE_TEXTURE_2D not for PIPE_TEXTURE_2D_ARRAY
> +        * Thought it might still be interessting to use hyperz for texture
> +        * array without using fast clear features
> +        */
> +       rtex->htile = NULL;
> +       if (!(base->flags & (R600_RESOURCE_FLAG_TRANSFER | R600_RESOURCE_FLAG_FLUSHED_DEPTH)) &&
> +           util_format_is_depth_or_stencil(base->format) &&
> +           rscreen->use_hyperz &&
> +           rscreen->info.drm_minor >= 14 &&

AFAIK there's no need to check for drm_minor here, because the
checking is already done in screen_create.

Marek