[igt-dev] [PATCH i-g-t] lib/i915/gem_mman.c: add cpu coherency mapping wrapper

[Chris Wilson]

Quoting Dixit, Ashutosh (2020-01-14 05:59:54)
> On Mon, 13 Jan 2020 21:27:28 -0800, Zbigniew Kempczyński wrote:
> >
> > For reduce code redundancy adding a wrapper for cpu memory mapping.
> >
> > +void *__gem_mmap__cpu_coherent(int fd, uint32_t handle, uint64_t offset,
> > +                            uint64_t size, unsigned prot)
> > +{
> > +     void *ptr = __gem_mmap_offset__cpu(fd, handle, offset, size, prot);
> > +
> > +     if (!ptr)
> > +             ptr = __gem_mmap__cpu(fd, handle, offset, size, prot);
> > +
> > +     return ptr;
> > +}
> 
> We need similar wrappers for WC and GTT too. So why don't we put this code

That's gem_mmap__device_coherent.

> in __gem_mmap__cpu() itself and we can do the same for __gem_mmap__wc() and
> __gem_mmap__gtt() too? Otherwise what are we going to call those functions?
> Something like:
> 
> void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
> {
>         if (gem_has_mmap_offset(fd))
>                 return __gem_mmap_offset(fd, handle, offset, size, prot, I915_MMAP_OFFSET_WB);
>         else
>                 return __gem_mmap(fd, handle, offset, size, prot, 0);
> }
> 
> So I am not sure of the point of introducing new wrappers, this code could
> just be put in the old existing wrappers.

The point is that we are separating intent from implementation. Where a
test is not looking at the mmap ioctl, but just wants access to a
buffer, how it wants to access that buffer is the important bit of
information.
-Chris