[Mesa-dev] [PATCH 2/4] anv/image: Simplify initialization of the isl_tiling
chad.versace at intel.com
Wed Jul 6 16:16:49 UTC 2016
On Fri 01 Jul 2016, Nanley Chery wrote:
> On Fri, Jul 01, 2016 at 02:24:19PM -0700, Chad Versace wrote:
> > I don't agree with this patch.
> > Locally, the patch look correct. But when you consider that
> > anv_image_create() is public within the driver, the patch makes the code
> > fragile. Pre-patch, if the caller of anv_image_create() sets
> > anv_image_create_info::vk_info::tiling and leaves
> > anv_image_create_info::isl_tiling_flags unset (which I believe should be
> > a valid combination), then anv_image_create() correctly converts the
> > VkImageTilingFlags to isl_tiling_flags. Post-patch, that's no longer the
> > case; anv_image_create() ignores its VkImageTiling input.
> Thanks for finding that bug.
> Your description has actually pointed out an issue in the current code:
> If an internal caller specifies
> anv_image_create_info::vk_info::tiling = VK_IMAGE_TILING_OPTIMAL
> and leaves anv_image_create_info::isl_tiling_flags unset, then
> anv_image_create() ignores the VkImageTiling input and causes ISL to
> fail image creation later.
> To solve this problem, I think we should define ::isl_tiling_flags to be a
> opt-in bit-mask which works with the requested ::vk_info::tiling to provide
> more specificity on the actual desired tiling. With this in mind, we can drop
> the last two hunks from the above patch and replace the first with the
> isl_tiling_flags_t tiling_flags =
> (pCreateInfo->tiling == VK_IMAGE_TILING_LINEAR ?
> ISL_TILING_LINEAR_BIT : ISL_TILING_ANY_MASK);
> if (anv_info->isl_tiling_flags)
> tiling_flags &= anv_info->isl_tiling_flags;
> assert (tiling_flags);
> What do you think?
Yes, I like that change.
More information about the mesa-dev