From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH libdrm] tests/util: Add support for meson module Date: Fri, 27 Jan 2017 08:47:14 +0100 Message-ID: <20170127074713.GB18548@ulmo.ba.sec> References: <1484747961-5418-1-git-send-email-narmstrong@baylibre.com> <20170119091955.GA29394@ulmo.ba.sec> <20170120161739.GA28228@ulmo.ba.sec> <20170126154931.GA27726@ulmo.ba.sec> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0488529059==" Return-path: Received: from hqemgate15.nvidia.com (hqemgate15.nvidia.com [216.228.121.64]) by gabe.freedesktop.org (Postfix) with ESMTPS id 52BFE6E2FB for ; Fri, 27 Jan 2017 07:47:23 +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: Emil Velikov Cc: ML dri-devel , Neil Armstrong List-Id: dri-devel@lists.freedesktop.org --===============0488529059== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="wzJLGUyc3ArbnUjN" Content-Disposition: inline --wzJLGUyc3ArbnUjN Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Jan 26, 2017 at 04:14:56PM +0000, Emil Velikov wrote: > On 26 January 2017 at 15:49, Thierry Reding wrote: > > On Fri, Jan 20, 2017 at 06:28:39PM +0000, Emil Velikov wrote: > >> On 20 January 2017 at 16:17, Thierry Reding wrote: > >> > On Fri, Jan 20, 2017 at 02:13:00PM +0000, Emil Velikov wrote: > >> >> On 19 January 2017 at 09:19, Thierry Reding wrote: > >> >> > On Wed, Jan 18, 2017 at 02:59:21PM +0100, Neil Armstrong wrote: > >> >> >> Add support for Amlogic Meson DRM driver merged for Linux 4.10. > >> >> >> > >> >> >> Signed-off-by: Neil Armstrong > >> >> >> --- > >> >> >> tests/util/kms.c | 1 + > >> >> >> 1 file changed, 1 insertion(+) > >> >> > > >> >> > Applied, thanks. > >> >> > > >> >> Was going to say "NACK use Thierry's helpers" but you've beat me to it. > >> >> As a Tl;DR: we _really_ want to stop using drmOpen* for anything that's KMS. > >> > > >> > I think it's still useful to have this helper to iterate over all > >> > supported driver because it enables easy testing with just a simple > >> > modetest. > >> > > >> > That said, for (almost) as long as I remember I've been using the -M > >> > option to prevent modetest from iterating over the list, which can take > >> > fairly long if you've got DRM_DEBUG messages enabled. > >> > > >> > I'm not sure I understand exactly what you're suggesting by "use > >> > Thierry's helpers". modetest and other tests use util_open() internally > >> > now. That in turn uses drmOpen(), though we're of course open to change > >> > that. Are you suggesting we somehow use drmDevice to locate existing > >> > devices? > >> > > >> Yes using drmDevice is what I had in mind. The only thing that was > >> stopping me from doing that is the lack of platform devices support. > >> With that in we can start purging _everything_ that uses drmOpen*. > >> > >> > We could implement some heuristic that finds the first device with a > >> > primary node, but what if we want to support the -M option? There is > >> > currently no way of getting the driver from drmDevice. Maybe that's > >> > something we should add anyway. > >> > > >> The -M (kernel module name as given by drmGetVersion) is another > >> interesting topic. Feel free to skip to the Tl;Dr below. > >> > >> If doing the ioctl implicitly via drmOpen or drmDevice wakes up the > >> device, keeping in mind that: > >> - it can take some time, and > >> - you don't always need the info > >> > >> A couple of elaborate workarounds include: > >> - use only on demand - add DRM_DEVICE_GET_MODULE_NAME or alike to the > >> drmDevice2 API > >> Needs a big "this can be VERY slow" warning in the documentation/man > >> pages... first we need actual man pages for libdrm ;-) > >> - deprecate the whole thing and use the compat strings - how do we > >> handle the PCI devices ? > >> - use the kernel module name (foo.ko) - kind of making it an ABI, but > >> iirc we've already have users which depend on it > >> Neither PCI devices nor platform ones are consistent in their naming - > >> foo.ko vs the value returned by the IOCTL. > >> > >> We might be able to combine the latter two ... need to double-check. > >> > >> Tl;Dr: For the moment I'd leave it to the user to call drmGetVersion. > > > > Okay, sounds like a plan. The good thing is we can keep adding new > > module names to the util_open() helper > The key point is that you do _not_ need any list. Any new and existing > drivers should just work. If they don't then they're [highly likely] > broken and should be fixed ;-) Yeah, what I was trying to say is that the implementation is now centralized, so adding module names until we replace the implementation isn't going to hurt much, while at the same time getting people the support that they want. Once we transition the implementation to using drmDevice, the list will simply disappear and everyone will be using the new code automatically. That said, I have a couple of things on my plate for today, but I'll go reimplement util_open() when I'm done with those. Thierry --wzJLGUyc3ArbnUjN Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAliK+wEACgkQ3SOs138+ s6GYqRAAhLSs6F7Zv3cPx+VdAd2w63tAgygHXws1c1WHHnhsnbuWIY37zQQnyTC2 50tDwmWFD721YwW6tA9AkHHfMFgUDSmdbgjhAt6R3+U5z61/Oi3puAV0ynmXG3Oa /bhTvMT0mjpd8gx4vQBrDe6BhZNWnqffYx4uc6q1Cb5nV69t0zdAVk2jAcRfMKSk WzBZB96GPvuY27n/23ijM4ZlJSEOUK90yflsUkm8gTAexsCmAGeVu/mDhC2ktEaW /geJAAOu87L6E8ZS8X0M6iDyqzWZ78lOLTI6Ihw2z0PaW0E+7+rTp7O9SFhnsZKY S+ZjpvnNG3iLLPg7lJH1SSDWaQ+pTYUmD1gQ7vJoV1Ae0U7i+EU1usBjHaCya8eu x0ipsI1J3ltROOLICiBpsIPjGtZGFHwZ9JFbwc1awDKJ3OYhtLD9d67XLYngIxEd 6VRmVKIW427eqvHvrXXC/PMUbvX0WChJYiCsn+H4uHW7srBJy6b5JAyZOqLe4pCA ztivID16U4d4IaXSFj4O5Izy3tbgzXseC4VjatIPcsIy5gZKMPbAFOH442BthI7e Ex+12/qoIhe7hhx4mXDKag/BLEIctP9T+/GpsqWT7TYt7/uOKlO2NmTVNdzQRHNi 5OMwkYY6ReaxCak0V9+ZAi1mcx0EEjtIoYvFBFzI8f7SX7mPOUU= =SAib -----END PGP SIGNATURE----- --wzJLGUyc3ArbnUjN-- --===============0488529059== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0488529059==--