Re: [PATCH linux vdagent] Add POC for getting xrandr output from monitor ID

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

adding Gerd to Cc.


On Fri, 2018-10-05 at 17:14 -0500, Jonathon Jongsma wrote:
> This is just a proof of concept utility that takes a PCI device address
> and a monitor ID and finds the xrandr output associated with that
> monitor id. It assumes that the list of drm connectors (outputs) are
> in a stable order and therefore we can use the monitor ID as an index
> into the array of connectors.
> ---
> 
> This isn't intended to be committed to the repository, but it gives an
> example for how we might do this. This will be necessary to handle vgpu
> streaming functionality properly.
> 
> To test, you can simply run the utility and pass a PCI address and
> monitor ID and it should print out the associated Xrandr output id. For
> example, on my laptop:
> 
> $ ./src/get-xrandr-output 0000:00:02.0 0
> Device /dev/dri/card0 is at ../../../0000:00:02.0
> DRM connector for monitor 0:	name=eDP-1 id=71 (*CONNECTED*)
>     Found matching X Output:	name=eDP-1 id=114 (*CONNECTED*)

I've tested it on my setup, it works (for QXL) even with PCI bridges,
which is good, but it doesn't work with virtio-gpu, which for some
reason has a different symlink. I did some rudimentary improvements to
the debugging messages, see a patch in reply to this email. Then, with
the following QEMU switches (you probably don't need the bridges to
reproduce):

-device pci-bridge,id=pci_bridge1,bus=pci.0,chassis_nr=1,addr=0x2 \
-device pci-bridge,id=pci_bridge2,bus=pci_bridge1,chassis_nr=1,addr=0x1 \
-device virtio-gpu,id=video0,bus=pci_bridge2,addr=0x3

I get this output:

# ./get-xrandr-output 0000:02:03.0 0
sys path: /sys/class/drm/card0
sys path device: /sys/class/drm/card0/device
Device /dev/dri/card0 is at ../../../virtio2
card addr 'virtio2' does not match requested addr '0000:02:03.0'
no card exists


Although, why do you go through the following process?
* looking up the card in /dev/dri
* finding the same card name in /sys under drm
* resolving the link to get the PCI address

Why not look up the PCI address directly in /sys and just verify the
address belongs to a drm device?


Also, as Frediano already mentioned in his email, in Gerd's initial
suggestion [1] the PCI address was like so:

  pci/$domain/$slot.$fn/$slot.$fn
                        ^^^^^^^^^  gpu device
              ^^^^^^^^^            pci bridge

I'm not saying we can't use the BDF, Gerd, did you have a reason for
not using the bus and list the PCI device hierarchy instead?

See also below...

[1] https://lists.freedesktop.org/archives/spice-devel/2018-August/045465.html


>  Makefile.am                     |  16 +++
>  configure.ac                    |   1 +
>  src/vdagent/get-xrandr-output.c | 209 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 226 insertions(+)
>  create mode 100644 src/vdagent/get-xrandr-output.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3e405bc..cdadb87 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -3,6 +3,7 @@ NULL =
>  
>  bin_PROGRAMS = src/spice-vdagent
>  sbin_PROGRAMS = src/spice-vdagentd
> +noinst_PROGRAMS = src/get-xrandr-output
>  
>  common_sources =				\
>  	src/udscs.c				\
> @@ -77,6 +78,21 @@ src_spice_vdagentd_SOURCES =			\
>  	src/vdagentd/virtio-port.h		\
>  	$(NULL)
>  
> +src_get_xrandr_output_SOURCES = \
> +	src/vdagent/get-xrandr-output.c \
> +	$(NULL)
> +
> +src_get_xrandr_output_CFLAGS = \
> +	$(AM_CPPFLAGS) \
> +	$(DRM_CFLAGS) \
> +	$(X_CFLAGS) \
> +	$(NULL)
> +
> +src_get_xrandr_output_LDADD = \
> +	$(DRM_LIBS) \
> +	$(X_LIBS) \
> +	$(NULL)
> +
>  if HAVE_CONSOLE_KIT
>  src_spice_vdagentd_SOURCES += src/vdagentd/console-kit.c
>  else
> diff --git a/configure.ac b/configure.ac
> index 7cb44db..55b031e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -105,6 +105,7 @@ PKG_CHECK_MODULES(X, [xfixes xrandr >= 1.3 xinerama x11])
>  PKG_CHECK_MODULES(SPICE, [spice-protocol >= 0.12.13])
>  PKG_CHECK_MODULES(ALSA, [alsa >= 1.0.22])
>  PKG_CHECK_MODULES([DBUS], [dbus-1])
> +PKG_CHECK_MODULES([DRM], [libdrm])
>  
>  if test "$with_session_info" = "auto" || test "$with_session_info" = "systemd"; then
>      PKG_CHECK_MODULES([LIBSYSTEMD_LOGIN],
> diff --git a/src/vdagent/get-xrandr-output.c b/src/vdagent/get-xrandr-output.c
> new file mode 100644
> index 0000000..2127e66
> --- /dev/null
> +++ b/src/vdagent/get-xrandr-output.c
> @@ -0,0 +1,209 @@
> +/* proof of concept for converting PCI address and monitor id to an xrandr
> + * output in the guest.
> + *
> + * \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/stat.h>
> +#include <xf86drm.h>
> +#include <xf86drmMode.h>
> +#include <unistd.h>
> +#include <X11/extensions/Xrandr.h>
> +
> +#define ERROR(...) \
> +    fprintf(stderr, __VA_ARGS__); \
> +    return EXIT_FAILURE;
> +
> +// connector type strings from kernel /drivers/gpu/drm/drm_connector.c
> +// these *should* match the drm Xrandr output names
> +static const char *conn_type[] = {
> +    [ DRM_MODE_CONNECTOR_Unknown] = "Unknown" ,
> +    [ DRM_MODE_CONNECTOR_VGA] = "VGA" ,
> +    [ DRM_MODE_CONNECTOR_DVII] = "DVI-I" ,
> +    [ DRM_MODE_CONNECTOR_DVID] = "DVI-D" ,
> +    [ DRM_MODE_CONNECTOR_DVIA] = "DVI-A" ,
> +    [ DRM_MODE_CONNECTOR_Composite] = "Composite" ,
> +    [ DRM_MODE_CONNECTOR_SVIDEO] = "SVIDEO" ,
> +    [ DRM_MODE_CONNECTOR_LVDS] = "LVDS" ,
> +    [ DRM_MODE_CONNECTOR_Component] = "Component" ,
> +    [ DRM_MODE_CONNECTOR_9PinDIN] = "DIN" ,
> +    [ DRM_MODE_CONNECTOR_DisplayPort] = "DP" ,
> +    [ DRM_MODE_CONNECTOR_HDMIA] = "HDMI-A" ,
> +    [ DRM_MODE_CONNECTOR_HDMIB] = "HDMI-B" ,
> +    [ DRM_MODE_CONNECTOR_TV] = "TV" ,
> +    [ DRM_MODE_CONNECTOR_eDP] = "eDP" ,
> +    [ DRM_MODE_CONNECTOR_VIRTUAL] = "Virtual" ,
> +    [ DRM_MODE_CONNECTOR_DSI] = "DSI" ,
> +    [ DRM_MODE_CONNECTOR_DPI] = "DPI" ,
> +};
> +
> +// FIXME: LICENSE?
> +// borrowed from Gerd's drminfo
> +void drm_conn_name(drmModeConnector *conn, char *dest, int dlen)
> +{
> +    const char *type;
> +
> +    if (conn->connector_type_id < sizeof(conn_type)/sizeof(conn_type[0]) &&
> +        conn_type[conn->connector_type]) {
> +        type = conn_type[conn->connector_type];
> +    } else {
> +        type = "unknown";
> +    }
> +    snprintf(dest, dlen, "%s-%d", type, conn->connector_type_id);
> +}
> +
> +static const char *connections[] = {"", "*CONNECTED*", "disconnected", "unknown connection"};
> +
> +int main(int argc, char* argv[])
> +{
> +    if (argc < 3) {
> +        ERROR("Usage: %s PCIADDR MONITORID", argv[0]);
> +    }
> +
> +    // PCI address should be in extended BDF notation
> +    // https://wiki.xen.org/wiki/Bus:Device.Function_(BDF)_Notation
> +    // FIXME: validate?
> +    char* user_addr = argv[1];
> +    int monitor_id = atoi(argv[2]);
> +
> +    bool found = false;
> +    for (int i = 0; i < 10; ++i) {
> +        char dev_path[64];
> +        struct stat buf;
> +
> +        // device node for the card is needed to access libdrm functionality
> +        snprintf(dev_path, sizeof(dev_path), DRM_DEV_NAME, DRM_DIR_NAME, i);
> +        if (stat(dev_path, &buf) != 0) {
> +            // no card exists, exit loop
> +            break;
> +        }
> +
> +        // the sysfs directory for the card will allow us to determine the
> +        // pci address for the device
> +        char sys_path[64];
> +        snprintf(sys_path, sizeof(sys_path), "/sys/class/drm/card%d", i);
> +
> +        // the file /sys/class/drm/card0/device is a symlink to a file that
> +        // specifies the device's address. It usually points to something
> +        // like ../../../0000:00:02.0
> +        char sys_path_device[100], device_link[100];
> +        snprintf(sys_path_device, sizeof(sys_path_device), "%s/device", sys_path);
> +        ssize_t res = readlink(sys_path_device, device_link, sizeof(device_link) - 1);
> +        if (res == -1) {
> +            ERROR(strerror(errno));
> +        }
> +        // readlink does not guarantee that the string is null-terminated
> +        device_link[res] = '\0';
> +        printf("Device %s is at %s\n", dev_path, device_link);
> +
> +        char *card_addr = device_link;
> +        // strip possible initial relative path
> +        char *delim = strrchr(device_link, '/');
> +        if (delim != NULL) {
> +            card_addr = delim + 1;
> +        }
> +
> +        if (strcmp(card_addr, user_addr) == 0) {
> +            int fd;
> +            fd = open(dev_path, O_RDWR);
> +            if (fd < 0) {
> +                ERROR(strerror(errno));
> +            }
> +
> +            drmModeResPtr res = drmModeGetResources(fd);
> +            if (!res) {
> +                ERROR("Unable to get resources for card");
> +            }
> +
> +            // find the drm output that is equal to monitor_id
> +            /* FIXME: verify that monitor id is 0-based */
> +            if (res->count_connectors < (monitor_id + 1)) {
> +                ERROR("Unable to find output");
> +            }
> +
> +            drmModeConnectorPtr conn = drmModeGetConnector(fd, res->connectors[monitor_id]);
> +            char cname[64];
> +            drm_conn_name(conn, cname, sizeof(cname));
> +            printf("DRM connector for monitor %i:	name=%s id=%u (%s)\n",
> +                   monitor_id, cname, conn->connector_id,
> +                   connections[conn->connection]);

I was actually thinking we would use the device_display_id (which you
call monitor_id here) to index directly the list of xrandr outputs, not
list the DRM connectors and then try to match the string name. (note in
that case it seems we wouldn't need libdrm at all?)

Is it possible these two won't contain the same outputs in the same
order? Gerd?

Cheers,
Lukas

> +            drmModeFreeConnector(conn);
> +            conn = NULL;
> +            drmModeFreeResources(res);
> +            res = NULL;
> +
> +            // now look up all xrandr outputs
> +            Display *xdisplay = XOpenDisplay(NULL);
> +            if (!xdisplay) {
> +                ERROR("Failed to open X dislay");
> +            }
> +
> +            int rr_event_base, rr_error_base;
> +            if (!XRRQueryExtension(xdisplay, &rr_event_base, &rr_error_base)) {
> +                ERROR("Failed to initialize XRandr extension");
> +            }
> +
> +            XRRScreenResources *xres = XRRGetScreenResourcesCurrent(xdisplay, DefaultRootWindow(xdisplay));
> +            if (!xres) {
> +                ERROR("Unable to get screen resources");
> +            }
> +
> +            // Loop through xrandr outputs and check whether the xrandr
> +            // output name matches the drm connector name
> +            bool increment_xrandr_name = false;
> +            for (int i = 0; i < xres->noutput; ++i) {
> +                int oid = xres->outputs[i];
> +                XRROutputInfo *oinfo = XRRGetOutputInfo(xdisplay, xres, oid);
> +                // the QXL device doesn't use the drm names directly, but
> +                // starts numbering from 0 instead of 1, so we need to
> +                // detect this condition and add 1 to all output names in
> +                // this scenario before comparing
> +                if (i == 0) {
> +                    // check the first output to see if it ends with '-0'
> +                    char *postfix = oinfo->name + (strlen(oinfo->name) - 2);
> +                    if (strcmp(postfix, "-0") == 0) {
> +                        increment_xrandr_name = true;
> +                    }
> +                }
> +                char xname[100];
> +                if (increment_xrandr_name) {
> +                    char *id_pos = strrchr(oinfo->name, '-') + 1;
> +                    int newid = atoi(id_pos) + 1;
> +                    if (snprintf(xname, sizeof(xname), "%s%i", oinfo->name, newid) < 0) {
> +                        ERROR("Unable to increment name");
> +                    }
> +                } else {
> +                    strncpy(xname, oinfo->name, sizeof(xname) - 1);
> +                }
> +                if (strcmp(xname, cname) == 0) {
> +                    printf("    Found matching X Output:	name=%s id=%i (%s)\n",
> +                           oinfo->name,
> +                           (int)oid,
> +                           connections[oinfo->connection + 1]);
> +                    XRRFreeOutputInfo(oinfo);
> +                    break;
> +                }
> +                XRRFreeOutputInfo(oinfo);
> +            }
> +
> +            XRRFreeScreenResources(xres);
> +            xres = NULL;
> +
> +            // we found the right card, abort the loop
> +            found = true;
> +            break;
> +        }
> +    }
> +    if (!found) {
> +        ERROR("Couldn't find a card with the given PCI address");
> +        return EXIT_SUCCESS;
> +    }
> +}
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]