On 30 January 2017 at 10:29, Thierry Reding <thierry.red...@gmail.com> wrote:
> From: Thierry Reding <tred...@nvidia.com>
>
> The util_open() helper is used in a couple of test programs to open an
> appropriate device. It takes a device path and a module name, both are
> optional, as parameters. If a device path is specified, it will try to
> open the given device. Otherwise it will try all available devices. If
> only a specific subset is desired, the module parameter can be used as
> a filter. The function will use it to open only devices whose kernel
> driver matches the given module name.
>
> Instead of relying on the legacy drmOpen() function to do this, convert
> util_open() to use the new drmDevice helpers. This gets it functionally
> much closer to what other DRM/KMS users, such as the X.Org Server or a
> Wayland server, do.
>
> Signed-off-by: Thierry Reding <tred...@nvidia.com>
> ---
> tests/util/kms.c | 167
> +++++++++++++++++++++++++++++++++++++++++--------------
> tests/util/kms.h | 43 ++++++++++++++
> 2 files changed, 168 insertions(+), 42 deletions(-)
>
> diff --git a/tests/util/kms.c b/tests/util/kms.c
> index d866398237bb..c5d35ab616d1 100644
> --- a/tests/util/kms.c
> +++ b/tests/util/kms.c
> +int util_get_devices(drmDevicePtr **devicesp, uint32_t flags)
Personally I would not have bothered with this helper, but it's nice to have ;-)
> +{
> + if (!devicesp)
> + return err;
> +
> +
> + if (devicesp)
With the "if !devicesp" check above this should always be true, no ?
> +int util_open_with_module(const char *device, const char *module)
Name and thus distinction vs util_open is blurred - here we require
non-null device... which we should check for.
Sadly I'm out of ideas for better name(s) :-(
> {
> - int fd;
> + int fd, err = 0;
> +
> + if (module)
> + printf("trying to open `%s' with `%s'...", device, module);
> + else
> + printf("trying to open `%s'...", device);
> +
> + fd = open(device, O_RDWR);
Not sure how much we should care here, but O_CLOEXEC would be nice.
> +int util_open(const char *device, const char *module)
> +{
> + } else {
> + fd = util_open_with_module(device, module);
Nit: one can drop a level of indentation/brackets - I have no strong
preference either way.
int util_open(...)
{
if (device)
return util_open_with_module(device, module);
...
get_devices
...
}
> --- a/tests/util/kms.h
> +++ b/tests/util/kms.h
> +
> +static inline char *util_device_get_node(drmDevicePtr device,
> + unsigned int type)
> +{
> + if (type >= DRM_NODE_MAX)
> + return NULL;
> +
IMHO it's better to honour the ::available_nodes bitmask here, since
we already check if we're within range.
This the above the series is
Reviewed-by: Emil Velikov <emil.l.veli...@gmail.com>
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
