[igt-dev] [PATCH i-g-t v3 01/13] lib/intel_bufops: add mapping on cpu / device

[Chris Wilson]

Quoting Zbigniew Kempczyński (2020-07-24 10:56:47)
> Simplify mapping intel_buf. To be extended with ref counting.
> Add intel_buf dump function for easy dump buffer to the file.
> Fixing returned type of bo size function.
> 
> Idempotency selftest is now skipped for default buf_ops creation
> to avoid time consuming comparison of HW and SW tiled buffers
> (this can be the problem for forked tests). Additional function
> buf_ops_create_with_selftest() was added to allow perform
> verification step where it is required.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Cc: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  lib/intel_bufops.c | 106 +++++++++++++++++++++++++++++++++++++++------
>  lib/intel_bufops.h |  15 ++++++-
>  2 files changed, 107 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c
> index 2a48fb0c..7ce540fb 100644
> --- a/lib/intel_bufops.c
> +++ b/lib/intel_bufops.c
> @@ -870,6 +870,48 @@ void intel_buf_destroy(struct intel_buf *buf)
>         free(buf);
>  }
>  
> +void *intel_buf_cpu_map(struct intel_buf *buf, bool write)
> +{
> +       int i915 = buf_ops_get_fd(buf->bops);
> +
> +       igt_assert(buf->ptr == NULL); /* already mapped */
> +
> +       buf->cpu_write = write;
> +       buf->ptr = gem_mmap__cpu_coherent(i915, buf->handle, 0,
> +                                         buf->surface[0].size,
> +                                         write ? PROT_WRITE : PROT_READ);
> +
> +       gem_set_domain(i915, buf->handle,
> +                      I915_GEM_DOMAIN_CPU,
> +                      write ? I915_GEM_DOMAIN_CPU : 0);
> +
> +       return buf->ptr;
> +}
> +
> +void *intel_buf_device_map(struct intel_buf *buf, bool write)
> +{
> +       int i915 = buf_ops_get_fd(buf->bops);
> +
> +       igt_assert(buf->ptr == NULL); /* already mapped */
> +
> +       buf->ptr = gem_mmap__device_coherent(i915, buf->handle, 0,
> +                                            buf->surface[0].size,
> +                                            write ? PROT_WRITE : PROT_READ);
> +

Why wouldn't you do the sync here if you did it in the other?

Bleugh. Push the synchronisation and flushing to the caller. I have a
strong dislike for it being magically applied, and believe such control
should be in the user. (I want to encourage creativity, for we find the
kernel bugs in the gaps between tests.)
-Chris