On Mon 01 Oct 2018, Jason Ekstrand wrote:
> The DRM format modifiers extension adds a TILING_DRM_FORMAT_MODIFIER
> which will be used for modifiers so we can no longer use OPTIMAL to
> indicate tiled inside the driver.
> ---
> src/intel/vulkan/anv_formats.c | 2 +-
> src/intel/vulkan/anv_image.c | 6 +++---
> src/intel/vulkan/genX_cmd_buffer.c | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
This patch needs to also fixup some places that test tiling ==
VK_IMAGE_TILING_LINEAR. I found at least one such place in
anv_formats.c. The patch also needs to fixup any functions that use
early returns that are conditioned (perhaps indirectly) on tiling;
anv_get_format_plane() comes to mind.
I quickly tried, as an experiment, to expand this patch into a correct
patch. After a few minutes of typing, I concluded that this patch series
takes the wrong order-of-implementation approach.
For example, what happens to all the calls to anv_get_format_plane() in
anv_blorp.c? Those need fixing too. Simply fixing the tiling checks is
not enough, especially because VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT
allows DRM_FORMAT_MOD_LINEAR.
Instead, I think the right way to do this is to incrementally, following
the callchains bottom-up, teach each function to understand
VK_EXT_image_drm_format_modifier. Only when that's complete, then move
WSI to the new structs. Otherwise, there is too much room for
accidential implementation gaps; gaps that may not hurt WSI, but will
make it more difficult to discern what-works-and-what-doesn't while
implementing the full extension.
Just now, I tried writing a patch that starts at the bottom of the
callchain, anv_get_format_plane(), and teaches it about modifiers. The
patch is more invasive than expected. Maybe the patch is messy because
more preliminary cleanups are needed. I'm unsure; I need to take a break
and revisit it in the morning.
>
> diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c
> index 33faf7cc37f..d44aae1c127 100644
> --- a/src/intel/vulkan/anv_formats.c
> +++ b/src/intel/vulkan/anv_formats.c
> @@ -508,7 +508,7 @@ get_image_format_features(const struct gen_device_info
> *devinfo,
> return 0;
>
> struct anv_format_plane base_plane_format = plane_format;
> - if (vk_tiling == VK_IMAGE_TILING_OPTIMAL) {
> + if (vk_tiling != VK_IMAGE_TILING_LINEAR) {
> base_plane_format = anv_get_format_plane(devinfo, vk_format,
> VK_IMAGE_ASPECT_COLOR_BIT,
> VK_IMAGE_TILING_LINEAR);
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index b0d8c560adb..70594d6c053 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -334,7 +334,7 @@ make_surface(const struct anv_device *dev,
> bool needs_shadow = false;
> if (dev->info.gen <= 8 &&
> (vk_info->flags & VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT) &&
> - vk_info->tiling == VK_IMAGE_TILING_OPTIMAL) {
> + vk_info->tiling != VK_IMAGE_TILING_LINEAR) {
> assert(isl_format_is_compressed(plane_format.isl_format));
> tiling_flags = ISL_TILING_LINEAR_BIT;
> needs_shadow = true;
> @@ -829,7 +829,7 @@ anv_layout_to_aux_usage(const struct gen_device_info *
> const devinfo,
> return ISL_AUX_USAGE_NONE;
>
> /* All images that use an auxiliary surface are required to be tiled. */
> - assert(image->tiling == VK_IMAGE_TILING_OPTIMAL);
> + assert(image->planes[plane].surface.isl.tiling != ISL_TILING_LINEAR);
>
> /* Stencil has no aux */
> assert(aspect != VK_IMAGE_ASPECT_STENCIL_BIT);
> @@ -953,7 +953,7 @@ anv_layout_to_fast_clear_type(const struct
> gen_device_info * const devinfo,
> return ANV_FAST_CLEAR_NONE;
>
> /* All images that use an auxiliary surface are required to be tiled. */
> - assert(image->tiling == VK_IMAGE_TILING_OPTIMAL);
> + assert(image->planes[plane].surface.isl.tiling != ISL_TILING_LINEAR);
>
> /* Stencil has no aux */
> assert(aspect != VK_IMAGE_ASPECT_STENCIL_BIT);
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 099c30f3d66..821506761e2 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -967,7 +967,7 @@ transition_color_buffer(struct anv_cmd_buffer *cmd_buffer,
> if (base_layer >= anv_image_aux_layers(image, aspect, base_level))
> return;
>
> - assert(image->tiling == VK_IMAGE_TILING_OPTIMAL);
> + assert(image->planes[plane].surface.isl.tiling != ISL_TILING_LINEAR);
>
> if (initial_layout == VK_IMAGE_LAYOUT_UNDEFINED ||
> initial_layout == VK_IMAGE_LAYOUT_PREINITIALIZED) {
> --
> 2.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
