From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2D0AD6E225 for ; Tue, 14 Jan 2020 05:59:56 +0000 (UTC) Date: Mon, 13 Jan 2020 21:59:54 -0800 Message-ID: <87eew2zig5.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20200114052728.13313-1-zbigniew.kempczynski@intel.com> References: <20200114052728.13313-1-zbigniew.kempczynski@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Subject: Re: [igt-dev] [PATCH i-g-t] lib/i915/gem_mman.c: add cpu coherency mapping wrapper List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-2" Content-Transfer-Encoding: quoted-printable Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Zbigniew =?ISO-8859-2?Q?Kempczy=F1ski?= Cc: igt-dev@lists.freedesktop.org List-ID: On Mon, 13 Jan 2020 21:27:28 -0800, Zbigniew Kempczy=F1ski 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 =3D __gem_mmap_offset__cpu(fd, handle, offset, size, prot); > + > + if (!ptr) > + ptr =3D __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 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 si= ze, 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. > + > +/** > + * gem_mmap__cpu_coherent: > + * @fd: open i915 drm file descriptor > + * @handle: gem buffer object handle > + * @offset: offset in the gem buffer of the mmap arena > + * @size: size of the mmap arena > + * @prot: memory protection bits as used by mmap() > + * > + * Call __gem_mmap__cpu__coherent(), asserts on fail. > + * Offset argument passed in function call must be 0. In the future > + * when driver will allow slice mapping of buffer object this restriction > + * will be removed. > + * > + * Returns: A pointer to the created memory mapping. > + */ > +void *gem_mmap__cpu_coherent(int fd, uint32_t handle, uint64_t offset, This can just be gem_mmap__cpu()? > + uint64_t size, unsigned prot) > +{ > + void *ptr; > + > + igt_assert(offset =3D=3D 0); Not needed? _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev