On Thu, Jan 26, 2017 at 04:14:56PM +0000, Emil Velikov wrote: > 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 ;-) 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
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
