[Mesa-dev] [PATCH] anv: Stop setting domains to RENDER on EXEC_OBJECT_WRITE

[Chris Wilson]

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 <chris at chris-wilson.co.uk>
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 <chris at chris-wilson.co.uk>
> ---
>  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 at chris-wilson.co.uk>
-Chris