From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Keith Packard" Subject: Re: [Mesa-dev] [PATCH 1/7] vulkan: Add KHR_display extension to anv and radv using DRM Date: Mon, 12 Feb 2018 15:12:32 -0800 Message-ID: <87y3jxajkv.fsf@keithp.com> References: <20180210044516.16944-1-keithp@keithp.com> <20180210044516.16944-2-keithp@keithp.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0514055641==" Return-path: 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: Emil Velikov Cc: ML mesa-dev , ML dri-devel List-Id: dri-devel@lists.freedesktop.org --===============0514055641== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Emil Velikov writes: > A few top level comments: Thanks much for looking over this code. I realize it's a large amount of change and hence more work to review. > - using card/primary node and (missing) authentication > Current code opens the primary node when VK_KHR_display is available. > Thus running a platform=3Ddrm,x11,wayland build will fail, as the client > is not authenticated with the X/WL server. this is really mostly there to make it easy to test the KHR_display backend without needing any further code. I added this only recently for this purpose. I'm not sure how we should be selecting for opening the primary device; perhaps another extension that adds a block in the pNext chain to look for? I don't know if you saw the first version of this series, but I had specified a new extension which provided the device FD from the application, letting you pass in whatever you could open (including a leased FD from X or a primary opened from the application). That seems like it could use some design ideas so we can make this do what we want. One option would be to remove the primary device code from the drivers once the leasing code is added so that the only way to use KHR_display is by leasing resources from the window system? The radv driver goes a bit further and checks with the kernel to see if rendering will work. For anv, the ioctls necessary for drawing are permitting on a non-authenticated node. Both drivers work fine for direct and X as-is, but I'm not very happy with having a random app with the primary device open as that seems like a mistake waiting to happen. > - reuse "drm" as a shorthand for the "display" > We've been using drm for egl/gbm, va/drm, etc, one less platform plus > no equivalent in either of egl/va/... Yeah, anything that supports drm can support display. I'll review the build process and try to simplify it further. > - remove conditional compilation based on library version/features. > Eg. checks like the following should be avoided. > #if DRM_EVENT_CONTEXT_VERSION Should I just have the build depend on a recent enough version of the libra= ry? > - use drmModeAddFB2 (or the withModifiers version even) over drmModeAddFB > Might help with the XXX just above it ;-) It won't help with that -- I just need more information from the driver. However, that information could come back as a format instead of depth/bpp, which would mean using drmModeAddFB2 instead of drmModeAddFB. Thanks for reminding me of this XXX. wsi_create_*_image gets a VkSwapchainCreateInfoKHR which has a VkFormat inside. Is that sufficient to compute a fourcc format for AddFB2? I think I could probably guess depth/bpp from it and be right most of the time? > - the spec says we're at VK_KHR_display rev 21, while the code advertise= s rev1 > Extension('VK_KHR_display', 1, > 'VK_USE_PLATFORM_DISPLAY_KHR'), Thanks. The version of the spec I've got (1.0.49) says it's already at rev 23! > - there are plenty of unnecessary of headers #include(d) As is often the case. I can go trim things down. > - we could simplify the ifdef spaghetti by making > wsi_*_{init,finish}_wsi empty stubs > Definitely something that can be done, independently of your work. That would be lovely. =2D-=20 =2Dkeith --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEw4O3eCVWE9/bQJ2R2yIaaQAAABEFAlqCH2EACgkQ2yIaaQAA ABGZwQ//ZAdGy7Yct/WZXAO0Dh5nfmfPT3BpxGYfhfbgFuVgK5BomzcVRA3nLDvz LkLYwQbVBUab7WnQ+1VVMy1ntavzkJf/OHZFRychI7V0vOCyy32mYL6XkLymvn4q xQxxw/159ouci62Xbh5hWLELrRtk3m6NqGXl+Q24fVxz/K4TFFZkvq5RqBvHiv9V QSyv9X5Y8DigPgI7W7joHsLc4hhcBtBnhu33sVW3SMv5ZzW3d3cs2Kv9vCHBVwgz r/c+E/nXtcKWo7iPXYXwLmUWDnnjgA6elu1/UgmHN9e111k9RDmpZC5UjYyets70 dZ/B+LsLcZ/C01br+iDrk0yasm3q1BnxC9oudyuiSNoZPx+VRTSTtx07LhgvDLtZ CDuJZgdocrlJzsDYbgAWhIJOFftezHYtAIET/nL7xCvS5+jKPelRm2AgT6tp0BWJ lIIrL+6hzGvtHXfhvYQZve8JG6ZmsKhwuIC2GhZj0JUiCOkmj/l/nQXhNuDs3BeG Uh3h1Ua+lTO6E24q60hbZ5m9bltrZjnROIgcHjScFmchfnnroKwi2ZrY5z6vNAmV 6lrmwHtttsXr3cm5zrpkx0Nk5ISnQvrX9z0fzSFkF7zaH78nxDYt/d0trnzPBji2 Qoo+9pPZePpbJ6cmrDOyeyBh+FfSLI0LEhkwk/ghMREzgaiVwaA= =mGUI -----END PGP SIGNATURE----- --=-=-=-- --===============0514055641== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0514055641==--