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: Wed, 06 Mar 2019 16:49:53 -0800 Message-ID: <871s3jtsni.fsf@anholt.net> References: <20190306230144.18135-1-robh@kernel.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0413985841==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id C575A6E21E for ; Thu, 7 Mar 2019 00:50:01 +0000 (UTC) In-Reply-To: <20190306230144.18135-1-robh@kernel.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Rob Herring , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0413985841== 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: > 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 correct > > 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=20 > panfrost which can be seen here[1]. I expect we'll be ready to merge=20 > panfrost for 5.2. 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. 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? I also hope we can plug this into vkms and turn on prime sharing for it. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlyAarEACgkQtdYpNtH8 nuhxPBAAj/2uR9hu/EJ0OkxATzTGP/oXo4XMyIfwK+H6f8CYEh55ODvKou9i0ZYi pYyuVgOussS975vMQqAuI2ckxrfYZPq5H+c5Tqas67ya7J2+rubTCRhwva75FjO+ UAsDAtmDaioAMpfqY8R7l6rEBRFWfhkFFhmrSawMDnJMLMYBhnRvtnxFh0VqND5z pKfnltAsr9XyL4VLjowJ/x0EpgLi7zfsvW1L+hfo1ZxwUuyfzvaUCaJN96BvlB9x WMMSiihAGVp2iI5SHCqFNYVoEtZbv3nXLv7r2SgNqsUZTRzQFtQQ4rv2mwmwZCqh 3oVwKGg36vYeGWqRIskNJy6gXR6GX/dMdrsAhAeqBawFDxF84qUYTubx0jVs9Aqo VkfWT5eBc3DRWw0DkmatZCRKe+ZyYZdpvyWw1lm0rbKZ4PIX9SpTcJFv1iTGJIrb qqUaN6mvZhcKagB5/pz4PT4zRBDxupyylOff/XenKGN+6bwCs1HqrLebAqUN9BFu ImSqjBrF7E7NIZzcnoSz4ii0wNKVxcJfv7JNsjcMQvhRm0QPdXMUzSACJeSrrK/K kH+LZOwXzDn5c/1o2ywkBXDV1rVUFrbmgg0xPSZONBiOMY+ak2wxzN40Ltl0/pkE 4w0uOAfQA6UEqdhXG9mJA8OmapvohvVpt5t2uYQ0P7zPO4z5fAs= =qkpR -----END PGP SIGNATURE----- --=-=-=-- --===============0413985841== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs --===============0413985841==--