* [PATCH libdrm] radeon, evergreen: ensure equal sizes for depth-stencil npot textures
@ 2018-09-10 16:48 Gert Wollny
0 siblings, 0 replies; 2+ messages in thread
From: Gert Wollny @ 2018-09-10 16:48 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Gert Wollny
From: Gert Wollny <gert.wollny@collabora.com>
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 some textures, namely npot and small (width < 32), but
not for mipmapped textures. In the problematic cases, 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 texture sizes in the problematic cases magnify
the depth texture alignment requirement by its bpe, so that the relative
alignment is the same for depth and stencil texture.
v2: * Also apply correction for pot textures with small width
* don't apply correction textures with more than one mipmap level
V3: * Apply correction for all small textures, even if they are mip-mapped
(makes one more piglit pass)
* Update comments
Fixes:
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
piglits:
arb_framebuffer_object-depth-stencil-blit *stencil*
framebuffer-blit-levels draw stencil
arb_texture_stencil8/
texwrap formats offset/gl_stencil_index8, npot
texwrap formats/gl_stencil_index8, npot
ext_framebuffer_multisample
accuracy all_samples stencil_resolve small depthstencil
unaligned-blit * stencil downsample
ext_texture_array/fbo-depth-array *stencil
egl_khr_gl_renderbuffer_image-clear-shared-image gl_depth_component24
Signed-off-by: Gert Wollny <gert.wollny@collabora.com>
---
I sent this before to dri-devel, but it seems that this list is more
apropriate.
Thanks for reviewing,
Gert
radeon/radeon_surface.c | 39 +++++++++++++++++++++++++++++++--------
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/radeon/radeon_surface.c b/radeon/radeon_surface.c
index 3cafcfcb..ff12f9f9 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,14 +795,26 @@ 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;
-
- r = eg_surface_init_1d(surf_man, surf, surf->level, surf->bpe, 0, 0);
+ (surf->flags & RADEON_SURF_HAS_SBUFFER_MIPTREE) ? surf->stencil_level : tmp;
+
+ /* 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 (The rules used here are deducted by
+ * running dEQP tests and piglits) */
+ int magnify_align = is_depth_stencil &&
+ ((surf->npix_x < 32) ||
+ (((surf->npix_x & (surf->npix_x - 1)) &&
+ !surf->last_level)));
+
+ 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,
+ r = eg_surface_init_1d(surf_man, surf, stencil_level, 1, 1,
surf->bo_size, 0);
surf->stencil_offset = stencil_level[0].offset;
}
@@ -817,13 +831,22 @@ static int eg_surface_init_2d_miptrees(struct radeon_surface_manager *surf_man,
struct radeon_surface_level *stencil_level =
(surf->flags & RADEON_SURF_HAS_SBUFFER_MIPTREE) ? surf->stencil_level : tmp;
+ /* 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. */
+ int magnify_align = is_depth_stencil &&
+ ((surf->npix_x < 32) ||
+ (((surf->npix_x & (surf->npix_x - 1)) &&
+ !surf->last_level)));
+
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,
+ 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;
}
--
2.16.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH libdrm] radeon, evergreen: ensure equal sizes for depth-stencil npot textures
2018-08-06 8:13 [PATCH] drm/radeon, " Gert Wollny
@ 2018-08-07 20:37 ` Gert Wollny
0 siblings, 0 replies; 2+ messages in thread
From: Gert Wollny @ 2018-08-07 20:37 UTC (permalink / raw)
To: dri-devel
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-09-10 16:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-10 16:48 [PATCH libdrm] radeon, evergreen: ensure equal sizes for depth-stencil npot textures Gert Wollny
-- strict thread matches above, loose matches on Subject: below --
2018-08-06 8:13 [PATCH] drm/radeon, " Gert Wollny
2018-08-07 20:37 ` [PATCH libdrm] radeon, " Gert Wollny
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.