[PATCH libdrm] tests/util: Add support for meson module

[Emil Velikov]

On 27 January 2017 at 07:47, Thierry Reding <treding at nvidia.com> wrote:
> On Thu, Jan 26, 2017 at 04:14:56PM +0000, Emil Velikov wrote:
>> On 26 January 2017 at 15:49, Thierry Reding <treding at nvidia.com> wrote:
>> > On Fri, Jan 20, 2017 at 06:28:39PM +0000, Emil Velikov wrote:
>> >> On 20 January 2017 at 16:17, Thierry Reding <treding at nvidia.com> wrote:
>> >> > On Fri, Jan 20, 2017 at 02:13:00PM +0000, Emil Velikov wrote:
>> >> >> On 19 January 2017 at 09:19, Thierry Reding <treding at nvidia.com> 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 at baylibre.com>
>> >> >> >> ---
>> >> >> >>  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.
>
Seems like I completely misunderstood you there :-\ Pardon if I came
too picky earlier.

Thanks
Emil