From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH v7] drm: Add library for shmem backed GEM objects Date: Thu, 07 Mar 2019 10:05:11 -0800 Message-ID: <87sgvytvag.fsf@anholt.net> References: <20190306230144.18135-1-robh@kernel.org> <871s3jtsni.fsf@anholt.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1430841253==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id 39B0289321 for ; Thu, 7 Mar 2019 18:05:15 +0000 (UTC) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Rob Herring Cc: dri-devel List-Id: dri-devel@lists.freedesktop.org --===============1430841253== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Rob Herring writes: > On Wed, Mar 6, 2019 at 6:50 PM Eric Anholt wrote: >> >> Rob Herring writes: >> >> > From: Noralf Tr=C3=B8nnes >> > >> > This adds a library for shmem backed GEM objects. >> > >> > v7: >> > - Use write-combine for mmap instead. This is the more common >> > case. (robher) >> > >> > v6: >> > - Fix uninitialized variable issue in an error path (anholt). >> > - Add a drm_gem_shmem_vm_open() to the fops to get proper refcounting >> > of the pages (anholt). >> > >> > v5: >> > - Drop drm_gem_shmem_prime_mmap() (Daniel Vetter) >> > - drm_gem_shmem_mmap(): Subtract drm_vma_node_start() to get the real >> > vma->vm_pgoff >> > - drm_gem_shmem_fault(): Use vmf->pgoff now that vma->vm_pgoff is corr= ect >> > >> > v4: >> > - Drop cache modes (Thomas Hellstrom) >> > - Add a GEM attached vtable >> > >> > v3: >> > - Grammar (Sam Ravnborg) >> > - s/drm_gem_shmem_put_pages_unlocked/drm_gem_shmem_put_pages_locked/ >> > (Sam Ravnborg) >> > - Add debug output in error path (Sam Ravnborg) >> > >> > Signed-off-by: Noralf Tr=C3=B8nnes >> > Signed-off-by: Eric Anholt >> > Signed-off-by: Rob Herring >> > --- >> > I don't have a user to submit just yet, but we are using this for >> > panfrost which can be seen here[1]. I expect we'll be ready to merge >> > panfrost for 5.2. > > I need to add exporting drm_gem_shmem_create_with_handle() as well. I > was thinking I was just using that in my rockchip conversion attempt, > but I need it for panfrost too. > >> Excellent. I'm taking a look at this for v3d, and I see that on the >> panfrost side you're allocating shmem->sgt and doing dma_map_sg() in >> your MMU map code, with no error handling. And, on MMU unmap I see >> dma_unmap_sg() unconditionally (won't that unbalance for a prime import >> which will presumably do its own unmapping?), but it also looks like the >> sgt is not freed. > > Indeed, I'll need to split those. > > Error handling? Panfrost doesn't do that. ;) > >> Can we do something nicer for handling the driver's desire for the sgt >> to fill its PTEs, regardless of where the BO came from? > > Not sure I follow. You mean do something in shmem helpers? I'm > assuming when we pin and dma map will evolve from at BO creation time > to on demand. Yeah, basically my driver wants not drm_gem_shmem_get_pages() as its interface but drm_shmem_get_sgt(). I'm imagining a helper for that which has its own refcount that does a get_pages and then, for non-dmabuf-import, the sgt allocation and dma mapping. >> I also hope we can plug this into vkms and turn on prime sharing for it. > > Doesn't seem too hard. I'd assume vkms would be okay with WC mappings > as it looks like they are currently cacheable? We could add a dma > attrs field to let users customize the mapping, but I'd rather avoid > that if possible. WC was my intent. It would make vkms look more like other drivers from a perf perspective, which I think would be reasonable (and then you don't have CPU cache issues when sharing buffers). I've got a patch laying around for converting to the helpers. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlyBXVcACgkQtdYpNtH8 nuiopQ/+PTxAsWph2rtBqcNY6N8pi7xjMNV/xAZTsKsFuhG6380ahqFFxmx7izOC c1yk0zSOUmG+lNs2reciTjTcySEGYz7fA2ufNha4fTXGhUelWk/+DJohTF7myOS8 9ku1eBC+1aezw8Bp3314I1UMIx/RCQ9rp/3dC4WH8PjmaLcIWy0+OBRWWJScR/i0 hlCliK1FuMESOU83XYyWFFokJhjPP5fA11Zni3TqQoRABujSh0OoEuk3ZQUF2v9m 7X2cUJ90SlSzc01L1MNfghDlN7ylvfOmQUbpOE57U/vkG0fM9bq9h5sz2Uh93T42 PP+wKET3pL1OxNWQJbGBmDHkvDq/20CCfUjh4D6Cze+zpf97pLx4ttftMFuQw6C/ lDJlve/PcSU9SJ+1oCnMnaa7PoAJSN5I9/RTiT+0zcjp7F2Z4AmT9jnDYNCESfXZ 1ahXARlE/o9D+4EOD1FRUvFpFB8arUa8LH1/wx8tj47lqMuNEGVtx74vqdA3cf6j 9MoE/y84U9Rdo0AZyh8ZQwiwiWxWhIAfV/4vKifX6unOSHFxr3kdiC3B7OEO9su2 XJu5dMCVsuEWTAPHqC3dqh9FFBqkOH4D1OY43P35PI5NrqAssj7MBHT7w06vAaBA +PtQ4/l6EjHxKYO09BUCmsdGbXybjDzR3a6+rNHibDVmMPFz88k= =1x+D -----END PGP SIGNATURE----- --=-=-=-- --===============1430841253== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs --===============1430841253==--