On 30 January 2017 at 10:29, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@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 treding@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.velikov@gmail.com
-Emil