Re: [PATCH libdrm] radeon, evergreen: ensure equal sizes for depth-stencil npot textures

[Gert Wollny <gert.wollny@collabora.com>]

It seems I did a wrong indication in the subject, 

Am Montag, den 06.08.2018, 10:13 +0200 schrieb Gert Wollny:
> On evergreen depth-stencil textures are allocated as two objects, and
> when using the eg_surface_init_1d_miptrees code path the size
> evaluation
> uses the generalized surf_minify function. Here when allocating the
> depth texture the alignment takes the depth bpe value into account,
> and
> uses bpe=1 for the stencil texture. As a result the texture pair may
> consist of textures with two different nblk_x sizes and this seems to
> be a problem with npot textures, i.e. if the so allocated depth
> texture
> is larger than the stencil texture, then the kernel may reject sent
> data
> with an error message like:
> 
>  evergreen_cs_track_validate_stencil:622 stencil read bo too
>   small (layer size 131072, offset 524288, max layer 1, bo size
> 606208)
> 
> because apparently the expected layer size is evaluated from the
> depth
> texture size, but the actual bo size is evaluated based on the true
> texture
> size values. If, on the other hand, the stencil texture is larger
> than the
> depth texture, then the data is send with a wrong alignment, and
> certain
> dEQP-GLES31 tests fail.
> 
> In order to obtain equal npot texture sizes magnify the depth texture
> alignment
> requirement in these cases by its bpe, so that the relative alignment
> is
> the same for depth and stencil texture.
> 
> Fixes on BARTS:
>   dEQP-GLES31.functional.stencil_texturing.format
>     .depth32f_stencil8_2d
>     .depth32f_stencil8_2d_array
>     .depth24_stencil8_2d Pass
>     .depth24_stencil8_2d_array
>     .stencil_index8_2d
>     .stencil_index8_2d_array
>     .depth32f_stencil8_draw
>     .depth24_stencil8_draw
> 
>   dEQP-GLES31.functional.texture.border_clamp.formats
>     .stencil_index8.nearest_size_npot
>     .depth24_stencil8_sample_stencil.nearest_size_npot
>     .depth32f_stencil8_sample_stencil.nearest_size_npot
> 
>   dEQP-
> GLES31.functional.texture.border_clamp.per_axis_wrap_mode.texture_2d
>     .uint_stencil.nearest.s_clamp_to_edge_t_clamp_to_border_npot
>     .uint_stencil.nearest.s_repeat_t_clamp_to_border_npot
>     .uint_stencil.nearest.s_mirrored_repeat_t_clamp_to_border_npot
> 
> Signed-off-by: Gert Wollny <gert.wollny@collabora.com>
> ---
> Thanks for reviewing and any comments,
> Gert
> 
> PS: - I have commit rights for mesa, I'm not sure whether this also
> includes drm.
>     - This is a resend, because I first tried with a mail address
> that was not 
>       subscribed to the list and the mail probably ended up in some
> moderation 
>       queue (sorry)
> 
>  radeon/radeon_surface.c | 38 +++++++++++++++++++++++++++-----------
>  1 file changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/radeon/radeon_surface.c b/radeon/radeon_surface.c
> index 3cafcfcb..0579389c 100644
> --- a/radeon/radeon_surface.c
> +++ b/radeon/radeon_surface.c
> @@ -612,6 +612,7 @@ static int eg_surface_init_1d(struct
> radeon_surface_manager *surf_man,
>                                struct radeon_surface *surf,
>                                struct radeon_surface_level *level,
>                                unsigned bpe,
> +                              unsigned align_maginify,
>                                uint64_t offset, unsigned start_level)
>  {
>      uint32_t xalign, yalign, zalign, tilew;
> @@ -620,7 +621,7 @@ static int eg_surface_init_1d(struct
> radeon_surface_manager *surf_man,
>      /* compute alignment */
>      tilew = 8;
>      xalign = surf_man->hw_info.group_bytes / (tilew * bpe * surf-
> >nsamples);
> -    xalign = MAX2(tilew, xalign);
> +    xalign = MAX2(tilew, xalign * align_maginify);
>      yalign = tilew;
>      zalign = 1;
>      if (surf->flags & RADEON_SURF_SCANOUT) {
> @@ -652,7 +653,8 @@ static int eg_surface_init_1d(struct
> radeon_surface_manager *surf_man,
>  static int eg_surface_init_2d(struct radeon_surface_manager
> *surf_man,
>                                struct radeon_surface *surf,
>                                struct radeon_surface_level *level,
> -                              unsigned bpe, unsigned tile_split,
> +                              unsigned bpe, unsigned align_magnify,
> +                              unsigned tile_split,
>                                uint64_t offset, unsigned start_level)
>  {
>      unsigned tilew, tileh, tileb;
> @@ -691,7 +693,7 @@ static int eg_surface_init_2d(struct
> radeon_surface_manager *surf_man,
>          level[i].mode = RADEON_SURF_MODE_2D;
>          eg_surf_minify(surf, level+i, bpe, i, slice_pt, mtilew,
> mtileh, mtileb, offset);
>          if (level[i].mode == RADEON_SURF_MODE_1D) {
> -            return eg_surface_init_1d(surf_man, surf, level, bpe,
> offset, i);
> +            return eg_surface_init_1d(surf_man, surf, level, bpe,
> align_magnify, offset, i);
>          }
>          /* level0 and first mipmap need to have alignment */
>          offset = surf->bo_size;
> @@ -793,16 +795,24 @@ static int eg_surface_init_1d_miptrees(struct
> radeon_surface_manager *surf_man,
>      /* Old libdrm_macros.headers didn't have stencil_level in it.
> This prevents crashes. */
>      struct radeon_surface_level tmp[RADEON_SURF_MAX_LEVEL];
>      struct radeon_surface_level *stencil_level =
> -        (surf->flags & RADEON_SURF_HAS_SBUFFER_MIPTREE) ? surf-
> >stencil_level : tmp;
> +          (surf->flags & RADEON_SURF_HAS_SBUFFER_MIPTREE) ? surf-
> >stencil_level : tmp;
>  
> -    r = eg_surface_init_1d(surf_man, surf, surf->level, surf->bpe,
> 0, 0);
> +    int magnify_align = is_depth_stencil && (surf->npix_x & (surf-
> >npix_x - 1));
> +
> +    /* With certain sizes the depth and the stencil texture end up
> being of
> +     * different block sizes which later results in wrong rendering
> for npot
> +     * textures. Inflate the alignment requirement for the depth
> surface by
> +     * its bpe in order to make it allocate a texture that is of the
> same
> +     * block size like the stencil texture */
> +    r = eg_surface_init_1d(surf_man, surf, surf->level, surf->bpe,
> +                           magnify_align ? surf->bpe : 1, 0, 0);
>      if (r)
>          return r;
>  
>      if (is_depth_stencil) {
> -        r = eg_surface_init_1d(surf_man, surf, stencil_level, 1,
> -                               surf->bo_size, 0);
> -        surf->stencil_offset = stencil_level[0].offset;
> +       r = eg_surface_init_1d(surf_man, surf, stencil_level, 1, 1,
> +                              surf->bo_size, 0);
> +       surf->stencil_offset = stencil_level[0].offset;
>      }
>      return r;
>  }
> @@ -816,16 +826,22 @@ static int eg_surface_init_2d_miptrees(struct
> radeon_surface_manager *surf_man,
>      struct radeon_surface_level tmp[RADEON_SURF_MAX_LEVEL];
>      struct radeon_surface_level *stencil_level =
>          (surf->flags & RADEON_SURF_HAS_SBUFFER_MIPTREE) ? surf-
> >stencil_level : tmp;
> +    int magnify_align = is_depth_stencil && (surf->npix_x & (surf-
> >npix_x - 1));
>  
> +    /* Inflate the alignment requirement for npot textures to insure
> that
> +     * stencil and depth texture have the same block size. Use this
> only in
> +     * the 1d code path that uses the non-specific minify. */
>      r = eg_surface_init_2d(surf_man, surf, surf->level, surf->bpe,
> +                           magnify_align ? surf->bpe : 1,
>                             surf->tile_split, 0, 0);
>      if (r)
>          return r;
>  
>      if (is_depth_stencil) {
> -        r = eg_surface_init_2d(surf_man, surf, stencil_level, 1,
> -                               surf->stencil_tile_split, surf-
> >bo_size, 0);
> -        surf->stencil_offset = stencil_level[0].offset;
> +       r = eg_surface_init_2d(surf_man, surf, stencil_level, 1, 1,
> +                              surf->stencil_tile_split, surf-
> >bo_size, 0);
> +       surf->stencil_offset = stencil_level[0].offset;
> +
>      }
>      return r;
>  }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel