[Mesa-dev] [PATCH 2/2] radv: allow image view to be create before memory is bound
Jason Ekstrand
jason at jlekstrand.net
Fri Nov 4 01:58:21 UTC 2016
Oh, this is definitely against the spec... From section 11.6 of the 1.0.32
spec:
> Non-sparse resources must be bound completely and contiguously to a single
> VkDeviceMemory object before the resource is passed as a parameter to any
> of the following operations:
>
> creating image or buffer views
> updating descriptor sets
> recording commands in a command buffer
>
> Once bound, the memory binding is immutable for the lifetime of the
resource.
Please do not implement this! Tell the app author to fix their bug instead.
On Thu, Nov 3, 2016 at 6:29 PM, Dave Airlie <airlied at gmail.com> wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> The spec doesn't seem to say this is illegal anywhere, it is
> unexpected, but I've got an app doing what appears to be this.
>
> This change splits out setting the base addresses to a separate
> function, if the image has no memory bound we wait until
> descriptor update time to bind the base addresses, after this
> point they should be immutable.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
> src/amd/vulkan/radv_descriptor_set.c | 2 ++
> src/amd/vulkan/radv_image.c | 66 ++++++++++++++++++++++++++----
> ------
> src/amd/vulkan/radv_private.h | 3 ++
> 3 files changed, 54 insertions(+), 17 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_descriptor_set.c b/src/amd/vulkan/radv_
> descriptor_set.c
> index a4a12bf..599699d 100644
> --- a/src/amd/vulkan/radv_descriptor_set.c
> +++ b/src/amd/vulkan/radv_descriptor_set.c
> @@ -616,6 +616,8 @@ write_image_descriptor(struct radv_device *device,
> const VkDescriptorImageInfo *image_info)
> {
> RADV_FROM_HANDLE(radv_image_view, iview, image_info->imageView);
> +
> + radv_set_descriptor_base_addresses(device, iview);
> memcpy(dst, iview->descriptor, 8 * 4);
> memcpy(dst + 8, iview->fmask_descriptor, 8 * 4);
> *buffer_list = iview->image->bo;
> diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c
> index 7e8b837..da8d0f5 100644
> --- a/src/amd/vulkan/radv_image.c
> +++ b/src/amd/vulkan/radv_image.c
> @@ -195,31 +195,63 @@ si_set_mutable_tex_desc_fields(struct radv_device
> *device,
> unsigned block_width, bool is_stencil,
> uint32_t *state)
> {
> - uint64_t gpu_address = device->ws->buffer_get_va(image->bo) +
> image->offset;
> - uint64_t va = gpu_address + base_level_info->offset;
> unsigned pitch = base_level_info->nblk_x * block_width;
>
> - state[1] &= C_008F14_BASE_ADDRESS_HI;
> state[3] &= C_008F1C_TILING_INDEX;
> state[4] &= C_008F20_PITCH;
> state[6] &= C_008F28_COMPRESSION_EN;
>
> - assert(!(va & 255));
> -
> - state[0] = va >> 8;
> - state[1] |= S_008F14_BASE_ADDRESS_HI(va >> 40);
> state[3] |= S_008F1C_TILING_INDEX(si_tile_mode_index(image,
> base_level,
> is_stencil));
> state[4] |= S_008F20_PITCH(pitch - 1);
>
> if (image->surface.dcc_size && image->surface.level[first_level].dcc_enabled)
> {
> state[6] |= S_008F28_COMPRESSION_EN(1);
> - state[7] = (gpu_address +
> - image->dcc_offset +
> - base_level_info->dcc_offset) >> 8;
> + state[7] = 0;
> }
> }
>
> +void
> +radv_set_descriptor_base_addresses(struct radv_device *device,
> + struct radv_image_view *iview)
> +{
> + uint32_t *state = iview->descriptor;
> + uint64_t gpu_address = device->ws->buffer_get_va(iview->image->bo)
> + iview->image->offset;
> + uint64_t va;
> + const struct radeon_surf_level *base_level_info;
> +
> + if (iview->descriptor_has_base_address)
> + return;
> +
> + if (iview->aspect_mask == VK_IMAGE_ASPECT_STENCIL_BIT)
> + base_level_info = &iview->image->surface.
> stencil_level[iview->base_mip];
> + else
> + base_level_info = &iview->image->surface.level[
> iview->base_mip];
> +
> + va = gpu_address + base_level_info->offset;
> +
> + state[0] = va >> 8;
> + state[1] &= C_008F14_BASE_ADDRESS_HI;
> + state[1] |= S_008F14_BASE_ADDRESS_HI(va >> 40);
> +
> + if (iview->image->surface.dcc_size &&
> + iview->image->surface.level[iview->base_mip].dcc_enabled) {
> + state[7] = (gpu_address + iview->image->dcc_offset +
> + base_level_info->dcc_offset) >> 8;
> + }
> +
> + if (iview->image->fmask.size) {
> + uint32_t *state = iview->fmask_descriptor;
> +
> + va = gpu_address + iview->image->fmask.offset;
> + state[0] = va >> 8;
> + state[1] &= C_008F14_BASE_ADDRESS_HI;
> + state[1] |= S_008F14_BASE_ADDRESS_HI(va >> 40);
> + }
> +
> + iview->descriptor_has_base_address = true;
> +}
> +
> static unsigned radv_tex_dim(VkImageType image_type, VkImageViewType
> view_type,
> unsigned nr_layers, unsigned nr_samples, bool
> is_storage_image)
> {
> @@ -347,10 +379,6 @@ si_make_texture_descriptor(struct radv_device
> *device,
> /* Initialize the sampler view for FMASK. */
> if (image->fmask.size) {
> uint32_t fmask_format;
> - uint64_t gpu_address = device->ws->buffer_get_va(
> image->bo);
> - uint64_t va;
> -
> - va = gpu_address + image->offset + image->fmask.offset;
>
> switch (image->samples) {
> case 2:
> @@ -367,9 +395,8 @@ si_make_texture_descriptor(struct radv_device *device,
> fmask_format = V_008F14_IMG_DATA_FORMAT_INVALID;
> }
>
> - fmask_state[0] = va >> 8;
> - fmask_state[1] = S_008F14_BASE_ADDRESS_HI(va >> 40) |
> - S_008F14_DATA_FORMAT(fmask_format) |
> + fmask_state[0] = 0;
> + fmask_state[1] = S_008F14_DATA_FORMAT(fmask_format) |
> S_008F14_NUM_FORMAT(V_008F14_IMG_NUM_FORMAT_UINT);
> fmask_state[2] = S_008F18_WIDTH(width - 1) |
> S_008F18_HEIGHT(height - 1);
> @@ -803,6 +830,11 @@ radv_image_view_init(struct radv_image_view *iview,
> is_stencil ?
> &image->surface.stencil_level[range->baseMipLevel] :
> &image->surface.level[range->baseMipLevel], range->baseMipLevel,
> range->baseMipLevel,
> image->surface.blk_w, is_stencil,
> iview->descriptor);
> +
> + /* set base addresses if we have a image memory binding */
> + if (iview->image->bo)
> + radv_set_descriptor_base_addresses(device, iview);
> +
> }
>
> void radv_image_set_optimal_micro_tile_mode(struct radv_device *device,
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index cfb913f..7dfc146 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -1028,6 +1028,7 @@ struct radv_image_view {
> uint32_t base_mip;
> VkExtent3D extent; /**< Extent of VkImageViewCreateInfo::baseMipLevel.
> */
>
> + bool descriptor_has_base_address;
> uint32_t descriptor[8];
> uint32_t fmask_descriptor[8];
> };
> @@ -1050,6 +1051,8 @@ void radv_image_view_init(struct radv_image_view
> *view,
> VkImageUsageFlags usage_mask);
> void radv_image_set_optimal_micro_tile_mode(struct radv_device *device,
> struct radv_image *image,
> uint32_t micro_tile_mode);
> +void radv_set_descriptor_base_addresses(struct radv_device *device,
> + struct radv_image_view *iview);
> struct radv_buffer_view {
> struct radeon_winsys_bo *bo;
> VkFormat vk_format;
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161103/e5bf7da4/attachment-0001.html>
More information about the mesa-dev
mailing list