From 3f8cebee09cc80fa83387d03bd2084af8b0f0b95 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 25 Mar 2021 15:17:08 +0100 Subject: [PATCH] panfrost: Move image states out of pan_image_layout The layout is supposed to encode image miplevels/surfaces layout, not the state data stored in the buffers. It doesn't matter for a gallium driver, since resources are expected to hold both a layout and a state, but Vulkan is a bit different. In Vulkan, the image state is explicitly passed by the user when starting a render pass (vkCmdBeginRenderPass()), and might evolve depending on the operation done in this render pass. This state is not effective until the command buffer is queued and executed. For these reasons, keeping the image state attached to the VkImage object is not an option, but we'd still like to re-use the layout and state objects, and all common helpers acting on those objects. Let's move the state bits out of the layout to make that possible. Signed-off-by: Boris Brezillon Reviewed-by: Alyssa Rosenzweig Part-of: --- src/gallium/drivers/panfrost/pan_cmdstream.c | 2 +- src/gallium/drivers/panfrost/pan_fragment.c | 2 +- src/gallium/drivers/panfrost/pan_job.c | 2 +- src/gallium/drivers/panfrost/pan_mfbd.c | 16 ++++++++-------- src/gallium/drivers/panfrost/pan_resource.c | 18 +++++++++--------- src/gallium/drivers/panfrost/pan_resource.h | 3 +++ src/panfrost/lib/pan_texture.h | 18 ++++++++++++------ 7 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_cmdstream.c b/src/gallium/drivers/panfrost/pan_cmdstream.c index b2b8b987be7..b1a9a4c6075 100644 --- a/src/gallium/drivers/panfrost/pan_cmdstream.c +++ b/src/gallium/drivers/panfrost/pan_cmdstream.c @@ -1390,7 +1390,7 @@ emit_image_attribs(struct panfrost_batch *batch, enum pipe_shader_type shader, if (image->shader_access & PIPE_IMAGE_ACCESS_WRITE) { flags |= PAN_BO_ACCESS_WRITE; unsigned level = is_buffer ? 0 : image->u.tex.level; - rsrc->layout.slices[level].initialized = true; + rsrc->state.slices[level].data_valid = true; } panfrost_batch_add_bo(batch, rsrc->bo, flags); diff --git a/src/gallium/drivers/panfrost/pan_fragment.c b/src/gallium/drivers/panfrost/pan_fragment.c index cbbb1a8895a..a36c6c0c8af 100644 --- a/src/gallium/drivers/panfrost/pan_fragment.c +++ b/src/gallium/drivers/panfrost/pan_fragment.c @@ -42,7 +42,7 @@ panfrost_initialize_surface( unsigned level = surf->u.tex.level; struct panfrost_resource *rsrc = pan_resource(surf->texture); - rsrc->layout.slices[level].initialized = true; + rsrc->state.slices[level].data_valid = true; } /* Generate a fragment job. This should be called once per frame. (According to diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c index 98bdb982736..e81e983bcbd 100644 --- a/src/gallium/drivers/panfrost/pan_job.c +++ b/src/gallium/drivers/panfrost/pan_job.c @@ -775,7 +775,7 @@ panfrost_load_surface(struct panfrost_batch *batch, struct pipe_surface *surf, u struct panfrost_resource *rsrc = pan_resource(surf->texture); unsigned level = surf->u.tex.level; - if (!rsrc->layout.slices[level].initialized) + if (!rsrc->state.slices[level].data_valid) return; if (!rsrc->damage.inverted_len) diff --git a/src/gallium/drivers/panfrost/pan_mfbd.c b/src/gallium/drivers/panfrost/pan_mfbd.c index 29286509460..0c595528b18 100644 --- a/src/gallium/drivers/panfrost/pan_mfbd.c +++ b/src/gallium/drivers/panfrost/pan_mfbd.c @@ -245,7 +245,7 @@ get_z_internal_format(struct panfrost_batch *batch) static void panfrost_mfbd_zs_crc_ext_set_bufs(struct panfrost_batch *batch, struct MALI_ZS_CRC_EXTENSION *ext, - struct pan_image_slice_layout **checksum_slice) + struct pan_image_slice_state **checksum_slice) { struct panfrost_device *dev = pan_device(batch->ctx->base.screen); @@ -259,7 +259,7 @@ panfrost_mfbd_zs_crc_ext_set_bufs(struct panfrost_batch *batch, unsigned level = c_surf->u.tex.level; struct pan_image_slice_layout *slice = &rsrc->layout.slices[level]; - *checksum_slice = slice; + *checksum_slice = &rsrc->state.slices[level]; ext->crc_row_stride = slice->crc.stride; if (rsrc->checksum_bo) @@ -387,7 +387,7 @@ panfrost_mfbd_zs_crc_ext_set_bufs(struct panfrost_batch *batch, static void panfrost_mfbd_emit_zs_crc_ext(struct panfrost_batch *batch, void *extp, - struct pan_image_slice_layout **checksum_slice) + struct pan_image_slice_state **checksum_slice) { pan_pack(extp, ZS_CRC_EXTENSION, ext) { ext.zs_clean_pixel_write_enable = true; @@ -558,7 +558,7 @@ panfrost_mfbd_fragment(struct panfrost_batch *batch, bool has_draws) rts = fb + MALI_MULTI_TARGET_FRAMEBUFFER_LENGTH; } - struct pan_image_slice_layout *checksum_slice = NULL; + struct pan_image_slice_state *checksum_slice = NULL; if (zs_crc_ext) panfrost_mfbd_emit_zs_crc_ext(batch, zs_crc_ext, &checksum_slice); @@ -588,8 +588,8 @@ panfrost_mfbd_fragment(struct panfrost_batch *batch, bool has_draws) MAX2(samples, 1); struct panfrost_resource *prsrc = pan_resource(surf->texture); - if (!checksum_slice) - prsrc->layout.slices[surf->u.tex.level].checksum_valid = false; + if (checksum_slice != &prsrc->state.slices[surf->u.tex.level]) + prsrc->state.slices[surf->u.tex.level].crc_valid = false; } } @@ -628,7 +628,7 @@ panfrost_mfbd_fragment(struct panfrost_batch *batch, bool has_draws) params.has_zs_crc_extension = !!zs_crc_ext; if (checksum_slice) { - bool valid = checksum_slice->checksum_valid; + bool valid = checksum_slice->crc_valid; bool full = !batch->minx && !batch->miny && batch->maxx == batch->key.width && batch->maxy == batch->key.height; @@ -640,7 +640,7 @@ panfrost_mfbd_fragment(struct panfrost_batch *batch, bool has_draws) * valid for next time. */ params.crc_write_enable = valid || full; - checksum_slice->checksum_valid |= full; + checksum_slice->crc_valid |= full; } } diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index f2a7df2a99c..96f6e6432d7 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -98,7 +98,7 @@ panfrost_resource_from_handle(struct pipe_screen *pscreen, } rsc->layout.slices[0].offset = whandle->offset; - rsc->layout.slices[0].initialized = true; + rsc->state.slices[0].data_valid = true; panfrost_resource_set_damage_region(NULL, &rsc->base, 0, NULL); if (panfrost_should_checksum(dev, rsc)) { @@ -941,7 +941,7 @@ panfrost_ptr_map(struct pipe_context *pctx, * from a pending batch XXX */ panfrost_flush_batches_accessing_bo(ctx, rsrc->bo, true); - if ((usage & PIPE_MAP_READ) && rsrc->layout.slices[level].initialized) { + if ((usage & PIPE_MAP_READ) && rsrc->state.slices[level].data_valid) { pan_blit_to_staging(pctx, transfer); panfrost_flush_batches_accessing_bo(ctx, staging->bo, true); panfrost_bo_wait(staging->bo, INT64_MAX, false); @@ -1039,7 +1039,7 @@ panfrost_ptr_map(struct pipe_context *pctx, transfer->map = ralloc_size(transfer, transfer->base.layer_stride * box->depth); assert(box->depth == 1); - if ((usage & PIPE_MAP_READ) && rsrc->layout.slices[level].initialized) { + if ((usage & PIPE_MAP_READ) && rsrc->state.slices[level].data_valid) { panfrost_load_tiled_image( transfer->map, bo->ptr.cpu + rsrc->layout.slices[level].offset, @@ -1070,7 +1070,7 @@ panfrost_ptr_map(struct pipe_context *pctx, * initialized (maybe), so be conservative */ if (usage & PIPE_MAP_WRITE) { - rsrc->layout.slices[level].initialized = true; + rsrc->state.slices[level].data_valid = true; panfrost_minmax_cache_invalidate(rsrc->index_cache, &transfer->base); } @@ -1103,7 +1103,7 @@ pan_resource_modifier_convert(struct panfrost_context *ctx, { 0, 0, 0, rsrc->base.width0, rsrc->base.height0, depth }; for (int i = 0; i <= rsrc->base.last_level; i++) { - if (!rsrc->layout.slices[i].initialized) + if (!rsrc->state.slices[i].data_valid) continue; blit.dst.resource = &tmp_rsrc->base; @@ -1174,7 +1174,7 @@ panfrost_ptr_unmap(struct pipe_context *pctx, struct panfrost_device *dev = pan_device(pctx->screen); if (transfer->usage & PIPE_MAP_WRITE) - prsrc->layout.slices[transfer->level].checksum_valid = false; + prsrc->state.slices[transfer->level].crc_valid = false; /* AFBC will use a staging resource. `initialized` will be set when the * fragment job is created; this is deferred to prevent useless surface @@ -1207,7 +1207,7 @@ panfrost_ptr_unmap(struct pipe_context *pctx, struct panfrost_bo *bo = prsrc->bo; if (transfer->usage & PIPE_MAP_WRITE) { - prsrc->layout.slices[transfer->level].initialized = true; + prsrc->state.slices[transfer->level].data_valid = true; if (prsrc->layout.modifier == DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED) { assert(transfer->box.depth == 1); @@ -1273,7 +1273,7 @@ panfrost_ptr_flush_region(struct pipe_context *pctx, transfer->box.x + box->x + box->width); } else { unsigned level = transfer->level; - rsc->layout.slices[level].initialized = true; + rsc->state.slices[level].data_valid = true; } } @@ -1308,7 +1308,7 @@ panfrost_generate_mipmap( assert(rsrc->bo); for (unsigned l = base_level + 1; l <= last_level; ++l) - rsrc->layout.slices[l].initialized = false; + rsrc->state.slices[l].data_valid = false; /* Beyond that, we just delegate the hard stuff. */ diff --git a/src/gallium/drivers/panfrost/pan_resource.h b/src/gallium/drivers/panfrost/pan_resource.h index cd780edde00..9d4a7d26055 100644 --- a/src/gallium/drivers/panfrost/pan_resource.h +++ b/src/gallium/drivers/panfrost/pan_resource.h @@ -55,6 +55,9 @@ struct panfrost_resource { /* Description of the resource layout */ struct pan_image_layout layout; + /* Image state */ + struct pan_image_state state; + /* Whether the modifier can be changed */ bool modifier_constant; diff --git a/src/panfrost/lib/pan_texture.h b/src/panfrost/lib/pan_texture.h index 76821f2ebef..e6b3ceeac12 100644 --- a/src/panfrost/lib/pan_texture.h +++ b/src/panfrost/lib/pan_texture.h @@ -70,12 +70,6 @@ struct pan_image_slice_layout { unsigned offset; unsigned stride; } crc; - - /* Has anything been written to this slice? */ - bool initialized; - - /* Is the checksum for this slice valid? */ - bool checksum_valid; }; struct pan_image_layout { @@ -85,6 +79,18 @@ struct pan_image_layout { unsigned array_stride; }; +struct pan_image_slice_state { + /* Is the checksum for this slice valid? */ + bool crc_valid; + + /* Has anything been written to this slice? */ + bool data_valid; +}; + +struct pan_image_state { + struct pan_image_slice_state slices[MAX_MIP_LEVELS]; +}; + struct pan_image { /* Format and size */ uint16_t width0, height0, depth0, array_size;