[PATCH weston v6] compositor-drm: Ignore non-KMS devices
Ucan, Emre (ADITG/SW1)
eucan at de.adit-jv.com
Fri Feb 17 07:43:24 UTC 2017
Hi Daniel,
It would be great to have this patch in weston 2.0.
We made a similar patch to fix this issue for Renesas Gen3 boards.
Best regards
Emre Ucan
Software Group I (ADITG/SW1)
Tel. +49 5121 49 6937
> -----Original Message-----
> From: wayland-devel [mailto:wayland-devel-
> bounces at lists.freedesktop.org] On Behalf Of Daniel Stone
> Sent: Dienstag, 14. Februar 2017 17:12
> To: wayland-devel at lists.freedesktop.org
> Subject: [PATCH weston v6] compositor-drm: Ignore non-KMS devices
>
> 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 <daniels at collabora.com>
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> Reported-by: Thierry Reding <treding at nvidia.com>
> Reported-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
> libweston/compositor-drm.c | 145
> ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 103 insertions(+), 42 deletions(-)
>
> This is now R-b, but I'm going to leave it on the list to decide whether
> or not we should ship it; it's awfully close to release day.
>
> v6: Elaborate comment further. Properly initialise and test for old FD.
> Use correct filename in all cases. Don't overwrite 'id' in corner
> cases.
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 2a80c6d79..f364648c7 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1506,36 +1506,15 @@ on_drm_input(int fd, uint32_t mask, void *data)
> }
>
> static int
> -init_drm(struct drm_backend *b, struct udev_device *device)
> +init_kms_caps(struct drm_backend *b)
> {
> - const char *filename, *sysnum;
> uint64_t cap;
> - int fd, ret;
> + int ret;
> clockid_t clk_id;
>
> - sysnum = udev_device_get_sysnum(device);
> - if (sysnum)
> - b->drm.id = atoi(sysnum);
> - if (!sysnum || b->drm.id < 0) {
> - weston_log("cannot get device sysnum\n");
> - return -1;
> - }
> -
> - filename = udev_device_get_devnode(device);
> - fd = weston_launcher_open(b->compositor->launcher, filename,
> O_RDWR);
> - if (fd < 0) {
> - /* Probably permissions error */
> - weston_log("couldn't open %s, skipping\n",
> - udev_device_get_devnode(device));
> - return -1;
> - }
> -
> - weston_log("using %s\n", filename);
> -
> - b->drm.fd = fd;
> - b->drm.filename = strdup(filename);
> + weston_log("using %s\n", b->drm.filename);
>
> - ret = drmGetCap(fd, DRM_CAP_TIMESTAMP_MONOTONIC, &cap);
> + ret = drmGetCap(b->drm.fd, DRM_CAP_TIMESTAMP_MONOTONIC,
> &cap);
> if (ret == 0 && cap == 1)
> clk_id = CLOCK_MONOTONIC;
> else
> @@ -1547,13 +1526,13 @@ init_drm(struct drm_backend *b, struct
> udev_device *device)
> return -1;
> }
>
> - ret = drmGetCap(fd, DRM_CAP_CURSOR_WIDTH, &cap);
> + ret = drmGetCap(b->drm.fd, DRM_CAP_CURSOR_WIDTH, &cap);
> if (ret == 0)
> b->cursor_width = cap;
> else
> b->cursor_width = 64;
>
> - ret = drmGetCap(fd, DRM_CAP_CURSOR_HEIGHT, &cap);
> + ret = drmGetCap(b->drm.fd, DRM_CAP_CURSOR_HEIGHT, &cap);
> if (ret == 0)
> b->cursor_height = cap;
> else
> @@ -2939,12 +2918,68 @@ session_notify(struct wl_listener *listener, void
> *data)
> };
> }
>
> +/**
> + * Determines whether or not a device is capable of modesetting. If
> successful,
> + * sets b->drm.fd and b->drm.filename to the opened device.
> + */
> +static bool
> +drm_device_is_kms(struct drm_backend *b, struct udev_device *device)
> +{
> + const char *filename = udev_device_get_devnode(device);
> + const char *sysnum = udev_device_get_sysnum(device);
> + drmModeRes *res;
> + int id, fd;
> +
> + if (!filename)
> + return false;
> +
> + fd = weston_launcher_open(b->compositor->launcher, filename,
> O_RDWR);
> + if (fd < 0)
> + return false;
> +
> + res = drmModeGetResources(fd);
> + if (!res)
> + goto out_fd;
> +
> + if (res->count_crtcs <= 0 || res->count_connectors <= 0 ||
> + res->count_encoders <= 0)
> + goto out_res;
> +
> + if (sysnum)
> + id = atoi(sysnum);
> + if (!sysnum || id < 0) {
> + weston_log("couldn't get sysnum for device %s\n",
> filename);
> + goto out_res;
> + }
> +
> + /* We can be called successfully on multiple devices; if we have,
> + * clean up old entries. */
> + if (b->drm.fd >= 0)
> + weston_launcher_close(b->compositor->launcher, b-
> >drm.fd);
> + free(b->drm.filename);
> +
> + b->drm.fd = fd;
> + b->drm.id = id;
> + b->drm.filename = strdup(filename);
> +
> + return true;
> +
> +out_res:
> + drmModeFreeResources(res);
> +out_fd:
> + weston_launcher_close(b->compositor->launcher, fd);
> + return false;
> +}
> +
> /*
> * Find primary GPU
> * Some systems may have multiple DRM devices attached to a single seat.
> This
> * function loops over all devices and tries to find a PCI device with the
> * boot_vga sysfs attribute set to 1.
> * If no such device is found, the first DRM device reported by udev is used.
> + * Devices are also vetted to make sure they are are capable of
> modesetting,
> + * rather than pure render nodes (GPU with no display), or pure
> + * memory-allocation devices (VGEM).
> */
> static struct udev_device*
> find_primary_gpu(struct drm_backend *b, const char *seat)
> @@ -2961,6 +2996,8 @@ find_primary_gpu(struct drm_backend *b, const
> char *seat)
> udev_enumerate_scan_devices(e);
> drm_device = NULL;
> udev_list_entry_foreach(entry, udev_enumerate_get_list_entry(e))
> {
> + bool is_boot_vga = false;
> +
> path = udev_list_entry_get_name(entry);
> device = udev_device_new_from_syspath(b->udev, path);
> if (!device)
> @@ -2977,20 +3014,46 @@ find_primary_gpu(struct drm_backend *b, const
> char *seat)
> "pci", NULL);
> if (pci) {
> id = udev_device_get_sysattr_value(pci,
> "boot_vga");
> - if (id && !strcmp(id, "1")) {
> - if (drm_device)
> - udev_device_unref(drm_device);
> - drm_device = device;
> - break;
> - }
> + if (id && !strcmp(id, "1"))
> + is_boot_vga = true;
> }
>
> - if (!drm_device)
> - drm_device = device;
> - else
> + /* If we already have a modesetting-capable device, and this
> + * device isn't our boot-VGA device, we aren't going to use
> + * it. */
> + if (!is_boot_vga && drm_device) {
> + udev_device_unref(device);
> + continue;
> + }
> +
> + /* Make sure this device is actually capable of modesetting;
> + * if this call succeeds, b->drm.{fd,filename} will be set,
> + * and any old values freed. */
> + if (!drm_device_is_kms(b, device)) {
> udev_device_unref(device);
> + continue;
> + }
> +
> + /* There can only be one boot_vga device, and we try to use
> it
> + * at all costs. */
> + if (is_boot_vga) {
> + if (drm_device)
> + udev_device_unref(drm_device);
> + drm_device = device;
> + break;
> + }
> +
> + /* Per the (!is_boot_vga && drm_device) test above, we
> only
> + * trump existing saved devices with boot-VGA devices, so if
> + * we end up here, this must be the first device we've seen.
> */
> + assert(!drm_device);
> + drm_device = device;
> }
>
> + /* If we're returning a device to use, we must have an open FD for
> + * it. */
> + assert(!!drm_device == (b->drm.fd >= 0));
> +
> udev_enumerate_unref(e);
> return drm_device;
> }
> @@ -3193,7 +3256,6 @@ drm_backend_create(struct weston_compositor
> *compositor,
> struct drm_backend *b;
> struct udev_device *drm_device;
> struct wl_event_loop *loop;
> - const char *path;
> const char *seat_id = default_seat;
> int ret;
>
> @@ -3203,6 +3265,8 @@ drm_backend_create(struct weston_compositor
> *compositor,
> if (b == NULL)
> return NULL;
>
> + b->drm.fd = -1;
> +
> /*
> * KMS support for hardware planes cannot properly synchronize
> * without nuclear page flip. Without nuclear/atomic, hw plane
> @@ -3246,9 +3310,8 @@ drm_backend_create(struct weston_compositor
> *compositor,
> weston_log("no drm device found\n");
> goto err_udev;
> }
> - path = udev_device_get_syspath(drm_device);
>
> - if (init_drm(b, drm_device) < 0) {
> + if (init_kms_caps(b) < 0) {
> weston_log("failed to initialize kms\n");
> goto err_udev_dev;
> }
> @@ -3283,7 +3346,7 @@ drm_backend_create(struct weston_compositor
> *compositor,
> b->connector = config->connector;
>
> if (create_outputs(b, drm_device) < 0) {
> - weston_log("failed to create output for %s\n", path);
> + weston_log("failed to create output for %s\n", b-
> >drm.filename);
> goto err_udev_input;
> }
>
> @@ -3292,8 +3355,6 @@ drm_backend_create(struct weston_compositor
> *compositor,
> if (!b->cursors_are_broken)
> compositor->capabilities |= WESTON_CAP_CURSOR_PLANE;
>
> - path = NULL;
> -
> loop = wl_display_get_event_loop(compositor->wl_display);
> b->drm_source =
> wl_event_loop_add_fd(loop, b->drm.fd,
> --
> 2.11.0
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list