Re: [Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Jason Ekstrand]

On Thu, Nov 29, 2018 at 9:43 AM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> Pipeline barriers inserted through vkCmdPipelineBarrier() should be
> taken into account when copying results.
>
> In the particular bug below, the results of the
> vkCmdCopyQueryPoolResults() command was being overwritten by the
> preceding vkCmdCopyBuffer() with a same destination buffer. This is
> because we copy the buffers using the 3D pipeline whereas we copy the
> query results using the command streamer. Those work in parallel
> unless synchronized.
>
> Signed-off-by: Lionel Landwerlin 
> Suggested-by: Jason Ekstrand 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/intel/vulkan/genX_query.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/intel/vulkan/genX_query.c b/src/intel/vulkan/genX_query.c
> index ce8757f2643..edf9962179a 100644
> --- a/src/intel/vulkan/genX_query.c
> +++ b/src/intel/vulkan/genX_query.c
> @@ -730,10 +730,9 @@ void genX(CmdCopyQueryPoolResults)(
> ANV_FROM_HANDLE(anv_buffer, buffer, destBuffer);
>
> if (flags & VK_QUERY_RESULT_WAIT_BIT) {
> -  anv_batch_emit(_buffer->batch, GENX(PIPE_CONTROL), pc) {
> - pc.CommandStreamerStallEnable = true;
> - pc.StallAtPixelScoreboard = true;
> -  }
> +  if (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)
> + cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
> +  genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
>

Unfortunately, I think we need to do it somewhat unconditionally.  More
specifically

if ((flags & VK_QUERY_RESULT_WAIT_BIT) ||
(cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)) {
   cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
   genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
}

Otherwise, we still have a write-after-write hazard if the client does a
vkCmdFillBuffer to zero out the destination prior to copying query pool
results into it.

--Jason


> }
>
> struct anv_address dest_addr = anv_address_add(buffer->address,
> destOffset);
> --
> 2.20.0.rc1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Józef Kucia]

On Thu, Nov 29, 2018 at 5:25 PM Lionel Landwerlin
 wrote:
> Pretty easy.
>
> Fetch http://source.winehq.org/git/vkd3d.git
>
> Usual autotool compilation.
>
> make tests/d3d12
>
> I commented the tests I wasn't interested in for to make it easier and using 
> vktrace to understand what's going on.

Yeah, it's easy. You just need a relatively recent version of widl in
the path, SPIR-V and Vulkan headers. Widl is usually installed
together with Wine. You can also build widl from wine.git (make
tools/widl), and pass WIDL=/path/to/widl to configure.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Lionel Landwerlin]

On 29/11/2018 16:15, Jason Ekstrand wrote:
On Thu, Nov 29, 2018 at 9:52 AM Józef Kucia > wrote:


On Thu, Nov 29, 2018 at 4:43 PM Lionel Landwerlin
mailto:lionel.g.landwer...@intel.com>> wrote:
>
> Pipeline barriers inserted through vkCmdPipelineBarrier() should be
> taken into account when copying results.
>
> In the particular bug below, the results of the
> vkCmdCopyQueryPoolResults() command was being overwritten by the
> preceding vkCmdCopyBuffer() with a same destination buffer. This is
> because we copy the buffers using the 3D pipeline whereas we
copy the
> query results using the command streamer. Those work in parallel
> unless synchronized.
>
> Signed-off-by: Lionel Landwerlin mailto:lionel.g.landwer...@intel.com>>
> Suggested-by: Jason Ekstrand mailto:ja...@jlekstrand.net>>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894

Thanks for the fix.

For what it's worth, we have more test failures on Anv. A lot failures
are related to clearing multisample array textures. See failures in
test_clear_render_target_view(), if you are interested.


Mind filing a few bugs?  Feel free to bucket them such as "multisample 
array texture clears".  How easy is it to run your tests?


Pretty easy.

Fetch http://source.winehq.org/git/vkd3d.git

Usual autotool compilation.

make tests/d3d12

I commented the tests I wasn't interested in for to make it easier and 
using vktrace to understand what's going on.



-

Lionel

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Jason Ekstrand]

On Thu, Nov 29, 2018 at 9:52 AM Józef Kucia  wrote:

> On Thu, Nov 29, 2018 at 4:43 PM Lionel Landwerlin
>  wrote:
> >
> > Pipeline barriers inserted through vkCmdPipelineBarrier() should be
> > taken into account when copying results.
> >
> > In the particular bug below, the results of the
> > vkCmdCopyQueryPoolResults() command was being overwritten by the
> > preceding vkCmdCopyBuffer() with a same destination buffer. This is
> > because we copy the buffers using the 3D pipeline whereas we copy the
> > query results using the command streamer. Those work in parallel
> > unless synchronized.
> >
> > Signed-off-by: Lionel Landwerlin 
> > Suggested-by: Jason Ekstrand 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
>
> Thanks for the fix.
>
> For what it's worth, we have more test failures on Anv. A lot failures
> are related to clearing multisample array textures. See failures in
> test_clear_render_target_view(), if you are interested.
>

Mind filing a few bugs?  Feel free to bucket them such as "multisample
array texture clears".  How easy is it to run your tests?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Józef Kucia]

On Thu, Nov 29, 2018 at 4:43 PM Lionel Landwerlin
 wrote:
>
> Pipeline barriers inserted through vkCmdPipelineBarrier() should be
> taken into account when copying results.
>
> In the particular bug below, the results of the
> vkCmdCopyQueryPoolResults() command was being overwritten by the
> preceding vkCmdCopyBuffer() with a same destination buffer. This is
> because we copy the buffers using the 3D pipeline whereas we copy the
> query results using the command streamer. Those work in parallel
> unless synchronized.
>
> Signed-off-by: Lionel Landwerlin 
> Suggested-by: Jason Ekstrand 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894

Thanks for the fix.

For what it's worth, we have more test failures on Anv. A lot failures
are related to clearing multisample array textures. See failures in
test_clear_render_target_view(), if you are interested.

Thanks,
Józef
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Lionel Landwerlin]

Pipeline barriers inserted through vkCmdPipelineBarrier() should be
taken into account when copying results.

In the particular bug below, the results of the
vkCmdCopyQueryPoolResults() command was being overwritten by the
preceding vkCmdCopyBuffer() with a same destination buffer. This is
because we copy the buffers using the 3D pipeline whereas we copy the
query results using the command streamer. Those work in parallel
unless synchronized.

Signed-off-by: Lionel Landwerlin 
Suggested-by: Jason Ekstrand 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
Cc: mesa-sta...@lists.freedesktop.org
---
 src/intel/vulkan/genX_query.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/intel/vulkan/genX_query.c b/src/intel/vulkan/genX_query.c
index ce8757f2643..edf9962179a 100644
--- a/src/intel/vulkan/genX_query.c
+++ b/src/intel/vulkan/genX_query.c
@@ -730,10 +730,9 @@ void genX(CmdCopyQueryPoolResults)(
ANV_FROM_HANDLE(anv_buffer, buffer, destBuffer);
 
if (flags & VK_QUERY_RESULT_WAIT_BIT) {
-  anv_batch_emit(_buffer->batch, GENX(PIPE_CONTROL), pc) {
- pc.CommandStreamerStallEnable = true;
- pc.StallAtPixelScoreboard = true;
-  }
+  if (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)
+ cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
+  genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
}
 
struct anv_address dest_addr = anv_address_add(buffer->address, destOffset);
-- 
2.20.0.rc1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev