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

[Thierry Reding <treding@nvidia.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3332 bytes --]

On Fri, Jan 20, 2017 at 06:28:39PM +0000, Emil Velikov wrote:
> On 20 January 2017 at 16:17, Thierry Reding <treding@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@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@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 irrespective of when we change
the implementation to use drmDevice instead of drmOpen() since it is
conveniently encapsulated in a single function across libdrm.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel