On 14 February 2017 at 13:49, Daniel Stone <[email protected]> wrote: > Given that we can have render-only devices, or vgem in a class of its > own, ignore any non-KMS devices in compositor-drm's device selection. > For x86 platforms, this is mostly a non-issue since we look at the udev > boot_vga issue, but other architectures which lack this, and have > multiple KMS devices present, will hit this. > > Signed-off-by: Daniel Stone <[email protected]> > Cc: Emil Velikov <[email protected]> > Reported-by: Thierry Reding <[email protected]> > Reported-by: Daniel Vetter <[email protected]> > --- > libweston/compositor-drm.c | 142 > +++++++++++++++++++++++++++++++-------------- > 1 file changed, 100 insertions(+), 42 deletions(-) > > v5: Document find_primary_gpu() and fix FD leaks if we happen to find a > suitable non-boot-VGA device before later finding a boot-VGA device. > Seems like my initial assumption of is_kms being optional was off.
There's a few minor comments below, but I'm split how much of a blocker they are. Either way: Reviewed-by: Emil Velikov <[email protected]> > + if (sysnum) > + b->drm.id = atoi(sysnum); Overwriting the .id in the error case seems wrong, since we'll stomp over the valid data from the previous device. > + if (!sysnum || b->drm.id < 0) { > + weston_log("couldn't get sysnum for device %s\n", > + b->drm.filename); "filename" only > + goto out_res; > + } > + > + /* We can be called successfully on multiple devices; if we have, > + * clean up old entries. */ > + if (b->drm.fd) Use "b->drm.fd >= 0" ? > + weston_launcher_close(b->compositor->launcher, b->drm.fd); > + free(b->drm.filename); > + One might need to check that the b-> fields are initialised properly. > + /* Make sure this device is actually capable of modesetting; > + * if this call succeeds, b->drm.{fd,filename} will be set. */ ... will be set and the old ones will be freed/closed. -Emil _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
