On 26 January 2017 at 15:49, Thierry Reding <treding@xxxxxxxxxx> wrote: > On Fri, Jan 20, 2017 at 06:28:39PM +0000, Emil Velikov wrote: >> On 20 January 2017 at 16:17, Thierry Reding <treding@xxxxxxxxxx> wrote: >> > On Fri, Jan 20, 2017 at 02:13:00PM +0000, Emil Velikov wrote: >> >> On 19 January 2017 at 09:19, Thierry Reding <treding@xxxxxxxxxx> 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 <narmstrong@xxxxxxxxxxxx> >> >> >> --- >> >> >> 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 ;-) -Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel