pan/layout: document and prepare to fix planar plane offset

This change:
1. Rename pan_image_wsi_layout to pan_image_layout_constraints.
1. Document that pan_image_layout_constraints::offset_B will be used to
   pass planar plane offset for native images as well. Update
   array_stride_B to exclude such offset accordingly.
2. Document that whether explicit layout is used further depends on if
   the passed wsi_row_pitch_B is non-zero or not. Updated the checks to
   base on the new boolean use_explicit_layout.
3. Update and document the intended slice offset_B behavior so that
   native and imported images are aligned.

No behavior change from this commit since the client (panvk) hasn't set
the pan_image_layout_constraints->offset_B for planar plane yet.

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/35299>
This commit is contained in:
Yiwei Zhang 2025-06-04 12:58:10 -07:00 committed by Marge Bot
parent 6690c74f6d
commit a9006ebde0
5 changed files with 89 additions and 52 deletions

View file

@ -159,9 +159,9 @@ panfrost_resource_from_handle(struct pipe_screen *pscreen,
: whandle->modifier;
enum mali_texture_dimension dim =
panfrost_translate_texture_dimension(templat->target);
struct pan_image_wsi_layout explicit_layout = {
struct pan_image_layout_constraints explicit_layout = {
.offset_B = whandle->offset,
.row_pitch_B = whandle->stride,
.wsi_row_pitch_B = whandle->stride,
.strict = dev->debug & PAN_DBG_STRICT_IMPORT,
};

View file

@ -241,13 +241,13 @@ pan_image_get_wsi_row_pitch(const struct pan_image_props *props,
}
static bool
wsi_row_pitch_to_row_stride(unsigned arch, const struct pan_image_props *props,
unsigned plane_idx,
const struct pan_image_wsi_layout *wsi_layout,
unsigned *row_stride_B)
wsi_row_pitch_to_row_stride(
unsigned arch, const struct pan_image_props *props, unsigned plane_idx,
const struct pan_image_layout_constraints *layout_constraints,
unsigned *row_stride_B)
{
unsigned row_align_mask, offset_align_mask, width_px;
unsigned wsi_row_pitch_B = wsi_layout->row_pitch_B;
unsigned wsi_row_pitch_B = layout_constraints->wsi_row_pitch_B;
enum pipe_format format = props->format;
uint64_t modifier = props->modifier;
@ -283,7 +283,7 @@ wsi_row_pitch_to_row_stride(unsigned arch, const struct pan_image_props *props,
* This is something we can't change without risking breaking existing
* users, so we enforce this explicit tile alignment only if we were
* asked to. */
if (wsi_layout->strict &&
if (layout_constraints->strict &&
(afbc_tile_payload_row_stride_B % afbc_tile_payload_size_B)) {
mesa_loge("WSI pitch is not aligned on an AFBC tile");
return false;
@ -335,7 +335,7 @@ wsi_row_pitch_to_row_stride(unsigned arch, const struct pan_image_props *props,
return false;
}
if (wsi_layout->offset_B & offset_align_mask) {
if (layout_constraints->offset_B & offset_align_mask) {
mesa_loge("WSI offset not properly aligned");
return false;
}
@ -363,15 +363,19 @@ pan_image_surface_offset(const struct pan_image_layout *layout, unsigned level,
}
bool
pan_image_layout_init(unsigned arch, const struct pan_image_props *props,
unsigned plane_idx,
const struct pan_image_wsi_layout *wsi_layout,
struct pan_image_layout *layout)
pan_image_layout_init(
unsigned arch, const struct pan_image_props *props, unsigned plane_idx,
const struct pan_image_layout_constraints *layout_constraints,
struct pan_image_layout *layout)
{
/* Use explicit layout only when wsi_row_pitch_B is non-zero */
const bool use_explicit_layout =
layout_constraints && layout_constraints->wsi_row_pitch_B;
/* Explicit stride only work with non-mipmap, non-array, single-sample
* 2D image without CRC.
*/
if (wsi_layout &&
if (use_explicit_layout &&
(props->extent_px.depth > 1 || props->nr_samples > 1 ||
props->array_size > 1 || props->dim != MALI_TEXTURE_DIMENSION_2D ||
props->nr_slices > 1 || props->crc))
@ -385,19 +389,16 @@ pan_image_layout_init(unsigned arch, const struct pan_image_props *props,
const int align_req_B =
afbc ? pan_afbc_header_row_stride_align(arch, props->format,
props->modifier)
: afrc
? pan_afrc_buffer_alignment_from_modifier(props->modifier)
: linear_or_tiled_row_align_req(arch, props->format, props->modifier);
unsigned wsi_row_stride_B = 0;
uint64_t offset_B = 0;
: (afrc ? pan_afrc_buffer_alignment_from_modifier(props->modifier)
: linear_or_tiled_row_align_req(arch, props->format,
props->modifier));
/* Mandate alignment */
if (wsi_layout) {
if (!wsi_row_pitch_to_row_stride(arch, props, plane_idx, wsi_layout,
&wsi_row_stride_B))
unsigned wsi_row_stride_B = 0;
if (use_explicit_layout) {
if (!wsi_row_pitch_to_row_stride(arch, props, plane_idx,
layout_constraints, &wsi_row_stride_B))
return false;
offset_B = wsi_layout->offset_B;
}
const unsigned fmt_blocksize_B =
@ -433,6 +434,7 @@ pan_image_layout_init(unsigned arch, const struct pan_image_props *props,
align_h_el *= pan_afbc_tile_size(props->modifier);
}
uint64_t offset_B = layout_constraints ? layout_constraints->offset_B : 0;
for (unsigned l = 0; l < props->nr_slices; ++l) {
struct pan_image_slice_layout *slice = &layout->slices[l];
@ -447,7 +449,7 @@ pan_image_layout_init(unsigned arch, const struct pan_image_props *props,
/* Align levels to cache-line as a performance improvement for
* linear/tiled and as a requirement for AFBC */
if (!wsi_layout)
if (!use_explicit_layout)
offset_B = ALIGN_POT(offset_B, pan_image_slice_align(props->modifier));
slice->offset_B = offset_B;
@ -465,12 +467,12 @@ pan_image_layout_init(unsigned arch, const struct pan_image_props *props,
row_stride_B = ALIGN_POT(row_stride_B, align_req_B);
}
if (wsi_layout && !afbc) {
if (use_explicit_layout && !afbc) {
/* Explicit stride should be rejected by wsi_row_pitch_to_row_stride()
* if it's too small. */
assert(wsi_row_stride_B >= row_stride_B);
if (!afrc || wsi_layout->strict)
if (!afrc || layout_constraints->strict)
row_stride_B = wsi_row_stride_B;
} else if (linear) {
/* Keep lines alignment on 64 byte for performance */
@ -487,11 +489,13 @@ pan_image_layout_init(unsigned arch, const struct pan_image_props *props,
/* Explicit stride should be rejected by wsi_row_pitch_to_row_stride()
* if it's too small. */
assert(!wsi_layout || wsi_row_stride_B >= slice->row_stride_B);
assert(!use_explicit_layout ||
wsi_row_stride_B >= slice->row_stride_B);
if (wsi_layout && wsi_layout->strict) {
if (use_explicit_layout && layout_constraints->strict) {
slice->row_stride_B = wsi_row_stride_B;
slice_one_size_B = (uint64_t)wsi_layout->row_pitch_B * effective_height_el;
slice_one_size_B = (uint64_t)layout_constraints->wsi_row_pitch_B *
effective_height_el;
}
slice->afbc.stride_sb =
@ -545,10 +549,17 @@ pan_image_layout_init(unsigned arch, const struct pan_image_props *props,
}
/* Arrays and cubemaps have the entire miptree duplicated */
layout->array_stride_B = ALIGN_POT(offset_B, 64);
if (wsi_layout) {
layout->data_size_B = offset_B - wsi_layout->offset_B;
layout->array_stride_B =
ALIGN_POT(offset_B - layout->slices[0].offset_B, 64);
if (use_explicit_layout) {
layout->data_size_B = offset_B - layout_constraints->offset_B;
} else {
/* Native images start from offset 0, and the planar plane offset has
* been at least 4K page aligned below. So the base level slice offset
* should always be the same with the plane offset.
*/
assert(!layout_constraints ||
layout_constraints->offset_B == layout->slices[0].offset_B);
layout->data_size_B = ALIGN_POT(
(uint64_t)layout->array_stride_B * (uint64_t)props->array_size, 4096);
}

View file

@ -22,6 +22,20 @@ extern "C" {
#define MAX_IMAGE_PLANES 3
struct pan_image_slice_layout {
/* Offset in bytes relative to the base bo bound.
*
* Unlike gallium, vulkan has to report explicit image subres layout which
* disallows to hide the planar plane offset into the bo mapping. So we let
* the slice offsets to include the plane offset of the native multi-planar
* images to be consistent with the imported ones via explicit layout info.
* Doing so allows us to use a single code path to correctly:
* - report image subres layout and memory requirement
* - bind image memory
*
* XXX However, for native non-disjoint multi-planar images in panvk, this
* offset_B is currently relative to the planar plane offset instead of base
* bo offset. To be fixed in the follow up.
*/
unsigned offset_B;
/* For AFBC images, the number of bytes between two rows of AFBC
@ -94,9 +108,19 @@ struct pan_image_layout {
uint64_t array_stride_B;
};
struct pan_image_wsi_layout {
struct pan_image_layout_constraints {
/*
* Plane offset in bytes
* - For native images, it's the planar plane offset.
* - For imported images, it's the user specified explicit offset.
*
* To be noted, this offset might be adjusted to choose an optimal alignment,
* unless the layout constraints are explicit (wsi_row_patch_B != 0).
*/
unsigned offset_B;
unsigned row_pitch_B;
/* Row pitch in bytes. Non-zero if layout is explicit. */
unsigned wsi_row_pitch_B;
bool strict;
};
@ -151,10 +175,10 @@ pan_image_mip_level_size(const struct pan_image_props *props,
return size;
}
bool pan_image_layout_init(unsigned arch, const struct pan_image_props *props,
unsigned plane_idx,
const struct pan_image_wsi_layout *wsi_layout,
struct pan_image_layout *layout);
bool pan_image_layout_init(
unsigned arch, const struct pan_image_props *props, unsigned plane_idx,
const struct pan_image_layout_constraints *layout_constraints,
struct pan_image_layout *layout);
static inline unsigned
pan_image_get_wsi_offset(const struct pan_image_layout *layout, unsigned level)

View file

@ -519,7 +519,7 @@ static unsigned archs[] = {4, 5, 6, 7, 9, 12, 13};
unsigned __export_row_pitch_B = \
pan_image_get_wsi_row_pitch(&iprops, __plane, &layout, 0); \
unsigned __export_offset_B = pan_image_get_wsi_offset(&layout, 0); \
EXPECT_TRUE(__export_row_pitch_B == (__wsi_layout)->row_pitch_B && \
EXPECT_TRUE(__export_row_pitch_B == (__wsi_layout)->wsi_row_pitch_B && \
__export_offset_B == (__wsi_layout)->offset_B) \
<< " mismatch between import and export for <format=" \
<< util_format_name(iprops.format) << ",plane=" << __plane \
@ -786,8 +786,8 @@ TEST(WSI, Import)
assert(default_row_pitch_B > row_align_req_B);
if (row_align_req_B > 1) {
struct pan_image_wsi_layout wsi_layout = {
.row_pitch_B = default_row_pitch_B + 1,
struct pan_image_layout_constraints wsi_layout = {
.wsi_row_pitch_B = default_row_pitch_B + 1,
.strict = true,
};
@ -796,9 +796,9 @@ TEST(WSI, Import)
}
if (offset_align_req_B > 1) {
struct pan_image_wsi_layout wsi_layout = {
struct pan_image_layout_constraints wsi_layout = {
.offset_B = 1,
.row_pitch_B = default_row_pitch_B,
.wsi_row_pitch_B = default_row_pitch_B,
.strict = true,
};
@ -807,23 +807,25 @@ TEST(WSI, Import)
}
/* Exact match. */
struct pan_image_wsi_layout wsi_layout = {
.row_pitch_B = default_row_pitch_B,
struct pan_image_layout_constraints wsi_layout = {
.wsi_row_pitch_B = default_row_pitch_B,
.strict = true,
};
EXPECT_IMPORT_SUCCESS(arch, &iprops, p, &wsi_layout, &layout,
"tightly packed lines");
wsi_layout.row_pitch_B = default_row_pitch_B + row_align_req_B;
wsi_layout.wsi_row_pitch_B =
default_row_pitch_B + row_align_req_B;
EXPECT_IMPORT_SUCCESS(arch, &iprops, p, &wsi_layout, &layout,
"lines with padding");
wsi_layout.row_pitch_B = default_row_pitch_B - row_align_req_B;
wsi_layout.wsi_row_pitch_B =
default_row_pitch_B - row_align_req_B;
EXPECT_IMPORT_FAIL(arch, &iprops, p, &wsi_layout, &layout,
"partially aliased lines");
wsi_layout.row_pitch_B = default_row_pitch_B;
wsi_layout.wsi_row_pitch_B = default_row_pitch_B;
wsi_layout.offset_B = offset_align_req_B;
EXPECT_IMPORT_SUCCESS(arch, &iprops, p, &wsi_layout, &layout,
"properly aligned offset");

View file

@ -237,11 +237,11 @@ panvk_image_init_layouts(struct panvk_image *image,
else
format = vk_format_get_plane_format(image->vk.format, plane);
struct pan_image_wsi_layout plane_layout;
struct pan_image_layout_constraints plane_layout;
if (explicit_info) {
plane_layout = (struct pan_image_wsi_layout){
plane_layout = (struct pan_image_layout_constraints){
.offset_B = explicit_info->pPlaneLayouts[plane].offset,
.row_pitch_B = explicit_info->pPlaneLayouts[plane].rowPitch,
.wsi_row_pitch_B = explicit_info->pPlaneLayouts[plane].rowPitch,
};
}