On Wed, Jun 21, 2017 at 09:04:01AM +0200, Iago Toral wrote:
> On Tue, 2017-06-20 at 12:21 -0700, Nanley Chery wrote:
> > On Mon, Jun 19, 2017 at 04:19:36PM -0700, Jason Ekstrand wrote:
> > > On Wed, Jun 14, 2017 at 3:06 PM, Nanley Chery <nanleych...@gmail.co
> > > m> wrote:
> > >
> > > > On Wed, Jun 14, 2017 at 09:32:22AM +0200, Iago Toral wrote:
> > > > > On Tue, 2017-06-13 at 11:41 -0700, Nanley Chery wrote:
> > > > > > For 3D image subresources undergoing a layout transition via
> > > > > > PipelineBarrier, we increase the number of fast-cleared
> > > > > > layers to
> > > > > > match
> > > > > > the intended behaviour of KHR_maintenance1. When such
> > > > > > subresources
> > > > > > undergo layout transitions between subpasses, we don't do
> > > > > > this to
> > > > > > avoid
> > > > > > failing incorrect CTS tests. Instead, unify the behaviour in
> > > > > > both
> > > > > > scenarios, and wait for the CTS tests to catch up. See CL
> > > > > > 1111 for
> > > > > > the
> > > > > > test fix.
> > > > > >
> > > > > > On SKL+, this causes 3 test failures under:
> > > > > > dEQP-VK.pipeline.render_to_image.3d.*
> > > > > >
> > > > > > Signed-off-by: Nanley Chery <nanley.g.ch...@intel.com>
> > > > > > ---
> > > > > > src/intel/vulkan/anv_blorp.c | 8 ++++----
> > > > > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/src/intel/vulkan/anv_blorp.c
> > > > > > b/src/intel/vulkan/anv_blorp.c
> > > > > > index 421f860428..ff3d7b126f 100644
> > > > > > --- a/src/intel/vulkan/anv_blorp.c
> > > > > > +++ b/src/intel/vulkan/anv_blorp.c
> > > > > > @@ -1478,12 +1478,12 @@ anv_image_ccs_clear(struct
> > > > > > anv_cmd_buffer
> > > > > > *cmd_buffer,
> > > > > >
> > > > > > /* Blorp likes to treat 2D_ARRAY and 3D the same. */
> > > > > > uint32_t blorp_base_layer, blorp_layer_count;
> > > > > > - if (view) {
> > > > > > - blorp_base_layer = view->base_array_layer;
> > > > > > - blorp_layer_count = view->array_len;
> > > > > > - } else if (image->type == VK_IMAGE_TYPE_3D) {
> > > > >
> > > > > Maybe add a comment referencing the requirement from
> > > > > VK_KHR_maintenance1 so it is clear why we ignore the view for
> > > > > 3D images
> > > > > here?
> > > > >
> > > >
> > > > Thank you for suggesting I add a comment. I actually meant to
> > > > double-check this before sending it out, but forgot. In the
> > > > process of
> > > > writing the comment, I discovered that the desired behaviour for
> > > > this
> > > > part of the extension is still being determined (Vulkan issue
> > > > #849).
> > > >
> > >
> > > Issue #849 was resolved today. This patch is correct.
> > >
> > > Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net>
> > >
> > > --Jason
> > >
> > >
> >
> > Thanks for the update and review! Given how recent the resolution is,
> > I
> > don't have any supporting specification text to reference in a code
> > comment so I plan to leave the patch as is.
>
> Ok, I think that is fine for now.
>
> If you think it is worth it, maybe add a comment saying that this is
> the result of an issue filed against against VK_KHR_maintenance1, so
> people know where it comes from.
>
> Iago
>
I'll update the comment to reference the issue number.
> > -Nanley
> >
> > > > > > + if (image->type == VK_IMAGE_TYPE_3D) {
> > > > > > blorp_base_layer = 0;
> > > > > > blorp_layer_count = extent.depth;
> > > > > > + } else if (view) {
> > > > > > + blorp_base_layer = view->base_array_layer;
> > > > > > + blorp_layer_count = view->array_len;
> > > > > > } else {
> > > > > > blorp_base_layer = subresourceRange-
> > > > > > >baseArrayLayer;
> > > > > > blorp_layer_count = anv_get_layerCount(image,
> > > > > > subresourceRange);
> > > >
> > > > _______________________________________________
> > > > 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
