Re: [Mesa-dev] [PATCH] anv: Stop setting domains to RENDER on EXEC_OBJECT_WRITE
On Sat, Jul 8, 2017 at 12:18 PM, Matt Turner wrote: > On Sat, Jul 8, 2017 at 11:05 AM, Jason Ekstrand > wrote: > > On July 7, 2017 1:52:54 PM Chris Wilson > wrote: > > > >> Quoting Jason Ekstrand (2017-07-07 21:37:29) > >>> > >>> The reason we were doing this was to ensure that the kernel did the > >>> appropriate cross-ring synchronization and flushing. However, the > >>> kernel only looks at EXEC_OBJECT_WRITE to determine whether or not to > >>> insert a fence. It only cares about the domain for determining whether > >>> or not it needs to clflush the BO before using it for scanout but the > >>> domain automatically gets set to RENDER internally by the kernel if > >>> EXEC_OBJECT_WRITE is set. > >> > >> > >> Once upon a time we also depended upon EXEC_OBJECT_WRITE for correct > >> swapout. That was until I saw what you were planning to do for anv. Hmm, > >> that puts the oldest kernel that might support anv as > >> > >> commit 51bc140431e233284660b1d22c47dec9ecdb521e [v4.3] > >> Author: Chris Wilson > >> Date: Mon Aug 31 15:10:39 2015 +0100 > >> > >> drm/i915: Always mark the object as dirty when used by the GPU > > > > > > I think we're probably ok there. We have a hard requirement on memfd > which > > I think landed in 4.6 though I could be wrong about that. > > No. memfd_create was added in 3.17. > Bah. I don't know why 4.6 is stuck in my brain as being important but it is. :-/ In any case, is there some way we can check for that commit? Otherwise, I think the only real thing we can do is just hope you don't swap and accept corruption if you do. :-( --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv: Stop setting domains to RENDER on EXEC_OBJECT_WRITE
On Sat, Jul 8, 2017 at 11:05 AM, Jason Ekstrand wrote: > On July 7, 2017 1:52:54 PM Chris Wilson wrote: > >> Quoting Jason Ekstrand (2017-07-07 21:37:29) >>> >>> The reason we were doing this was to ensure that the kernel did the >>> appropriate cross-ring synchronization and flushing. However, the >>> kernel only looks at EXEC_OBJECT_WRITE to determine whether or not to >>> insert a fence. It only cares about the domain for determining whether >>> or not it needs to clflush the BO before using it for scanout but the >>> domain automatically gets set to RENDER internally by the kernel if >>> EXEC_OBJECT_WRITE is set. >> >> >> Once upon a time we also depended upon EXEC_OBJECT_WRITE for correct >> swapout. That was until I saw what you were planning to do for anv. Hmm, >> that puts the oldest kernel that might support anv as >> >> commit 51bc140431e233284660b1d22c47dec9ecdb521e [v4.3] >> Author: Chris Wilson >> Date: Mon Aug 31 15:10:39 2015 +0100 >> >> drm/i915: Always mark the object as dirty when used by the GPU > > > I think we're probably ok there. We have a hard requirement on memfd which > I think landed in 4.6 though I could be wrong about that. No. memfd_create was added in 3.17. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv: Stop setting domains to RENDER on EXEC_OBJECT_WRITE
On July 7, 2017 1:52:54 PM Chris Wilson wrote: Quoting Jason Ekstrand (2017-07-07 21:37:29) The reason we were doing this was to ensure that the kernel did the appropriate cross-ring synchronization and flushing. However, the kernel only looks at EXEC_OBJECT_WRITE to determine whether or not to insert a fence. It only cares about the domain for determining whether or not it needs to clflush the BO before using it for scanout but the domain automatically gets set to RENDER internally by the kernel if EXEC_OBJECT_WRITE is set. Once upon a time we also depended upon EXEC_OBJECT_WRITE for correct swapout. That was until I saw what you were planning to do for anv. Hmm, that puts the oldest kernel that might support anv as commit 51bc140431e233284660b1d22c47dec9ecdb521e [v4.3] Author: Chris Wilson Date: Mon Aug 31 15:10:39 2015 +0100 drm/i915: Always mark the object as dirty when used by the GPU I think we're probably ok there. We have a hard requirement on memfd which I think landed in 4.6 though I could be wrong about that. Cc: Chris Wilson --- src/intel/vulkan/anv_batch_chain.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index 9def174..9776a45 100644 --- a/src/intel/vulkan/anv_batch_chain.c +++ b/src/intel/vulkan/anv_batch_chain.c @@ -148,9 +148,6 @@ anv_reloc_list_add(struct anv_reloc_list *list, struct drm_i915_gem_relocation_entry *entry; int index; - const uint32_t domain = - (target_bo->flags & EXEC_OBJECT_WRITE) ? I915_GEM_DOMAIN_RENDER : 0; - VkResult result = anv_reloc_list_grow(list, alloc, 1); if (result != VK_SUCCESS) return result; @@ -163,8 +160,8 @@ anv_reloc_list_add(struct anv_reloc_list *list, entry->delta = delta; entry->offset = offset; entry->presumed_offset = target_bo->offset; - entry->read_domains = domain; - entry->write_domain = domain; + entry->read_domains = 0; + entry->write_domain = 0; The first time I saw this I was amazed we let 0 through. It is true that the kernel only cares about EXEC_OBJECT_WRITE, and doesn't care whether that is from an execobject.flag or from accumulation of reloc[].write_domain. (That has been true for all kernels since the introduction of NORELOC and the EXEC_OBJECT_WRITE flag) We don't even use the reloc.write_domain information during reloc itself, so Reviewed-by: Chris Wilson -Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv: Stop setting domains to RENDER on EXEC_OBJECT_WRITE
Quoting Jason Ekstrand (2017-07-07 21:37:29) > The reason we were doing this was to ensure that the kernel did the > appropriate cross-ring synchronization and flushing. However, the > kernel only looks at EXEC_OBJECT_WRITE to determine whether or not to > insert a fence. It only cares about the domain for determining whether > or not it needs to clflush the BO before using it for scanout but the > domain automatically gets set to RENDER internally by the kernel if > EXEC_OBJECT_WRITE is set. Once upon a time we also depended upon EXEC_OBJECT_WRITE for correct swapout. That was until I saw what you were planning to do for anv. Hmm, that puts the oldest kernel that might support anv as commit 51bc140431e233284660b1d22c47dec9ecdb521e [v4.3] Author: Chris Wilson Date: Mon Aug 31 15:10:39 2015 +0100 drm/i915: Always mark the object as dirty when used by the GPU > Cc: Chris Wilson > --- > src/intel/vulkan/anv_batch_chain.c | 7 ++- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/src/intel/vulkan/anv_batch_chain.c > b/src/intel/vulkan/anv_batch_chain.c > index 9def174..9776a45 100644 > --- a/src/intel/vulkan/anv_batch_chain.c > +++ b/src/intel/vulkan/anv_batch_chain.c > @@ -148,9 +148,6 @@ anv_reloc_list_add(struct anv_reloc_list *list, > struct drm_i915_gem_relocation_entry *entry; > int index; > > - const uint32_t domain = > - (target_bo->flags & EXEC_OBJECT_WRITE) ? I915_GEM_DOMAIN_RENDER : 0; > - > VkResult result = anv_reloc_list_grow(list, alloc, 1); > if (result != VK_SUCCESS) >return result; > @@ -163,8 +160,8 @@ anv_reloc_list_add(struct anv_reloc_list *list, > entry->delta = delta; > entry->offset = offset; > entry->presumed_offset = target_bo->offset; > - entry->read_domains = domain; > - entry->write_domain = domain; > + entry->read_domains = 0; > + entry->write_domain = 0; The first time I saw this I was amazed we let 0 through. It is true that the kernel only cares about EXEC_OBJECT_WRITE, and doesn't care whether that is from an execobject.flag or from accumulation of reloc[].write_domain. (That has been true for all kernels since the introduction of NORELOC and the EXEC_OBJECT_WRITE flag) We don't even use the reloc.write_domain information during reloc itself, so Reviewed-by: Chris Wilson -Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] anv: Stop setting domains to RENDER on EXEC_OBJECT_WRITE
The reason we were doing this was to ensure that the kernel did the appropriate cross-ring synchronization and flushing. However, the kernel only looks at EXEC_OBJECT_WRITE to determine whether or not to insert a fence. It only cares about the domain for determining whether or not it needs to clflush the BO before using it for scanout but the domain automatically gets set to RENDER internally by the kernel if EXEC_OBJECT_WRITE is set. Cc: Chris Wilson --- src/intel/vulkan/anv_batch_chain.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index 9def174..9776a45 100644 --- a/src/intel/vulkan/anv_batch_chain.c +++ b/src/intel/vulkan/anv_batch_chain.c @@ -148,9 +148,6 @@ anv_reloc_list_add(struct anv_reloc_list *list, struct drm_i915_gem_relocation_entry *entry; int index; - const uint32_t domain = - (target_bo->flags & EXEC_OBJECT_WRITE) ? I915_GEM_DOMAIN_RENDER : 0; - VkResult result = anv_reloc_list_grow(list, alloc, 1); if (result != VK_SUCCESS) return result; @@ -163,8 +160,8 @@ anv_reloc_list_add(struct anv_reloc_list *list, entry->delta = delta; entry->offset = offset; entry->presumed_offset = target_bo->offset; - entry->read_domains = domain; - entry->write_domain = domain; + entry->read_domains = 0; + entry->write_domain = 0; VG(VALGRIND_CHECK_MEM_IS_DEFINED(entry, sizeof(*entry))); return VK_SUCCESS; -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
