[PATCH xserver 4/4] xfree86: promote one GPU screen if (NumScreens == 0 && NumGPUScreens > 0)

[Hans de Goede]

HI,

On 04-09-16 03:11, Laszlo Ersek wrote:
> Aarch64/KVM virtual machines cannot use emulated graphics cards with
> linear framebuffers, due to architectural cache coherency issues. (For
> this reason, "qemu-system-aarch64" doesn't even include the "virtio-vga"
> device model.)
>
> Such guests can use the "virtio-gpu-pci" device well, through the
> "modesetting" driver. However, given that "virtio-gpu-pci" is never
> recognized as a primary graphics card (which is otherwise correct,
> generally speaking),

Why is this otherwise correct, generally speaking ? AFAIK virtio-gpu-pci
is needed for Virgil, so I would expect it to become the default graphical
card in vms in the future, or is that what virtio-vga is for ?

Either way I've a feeling that it would be better to just get
virtio-gpu-pci recgonized as primary card rather then doing
this moving a gpu from one list to the next trickery, which
is likely to break.

Actually it is already broken, the PLATFORM_PROBE_GPU_SCREEN flag
gets passed into the driver's probe method and at least the
modesetting driver behaves differently depending on whether
that flag as passed in or not. Now this only influences its
prime functionality which is not likely to get used in your
example, but it does show that simply moving the gpu from
one list to another is doing things to late. The fix need
to influence the Xserver's behavior before the drivers
probe method gets called.

I would try to recognize there is no primary card before any
probe methods are called and then specifically look for a
virtio-gpu-pci device and promote that to primary.

Note I would not just take any non primary device, that
may have unwanted consequences on actual hardware.

Regards,

Hans


p.s.

This also means that you can drop patch 3/4 from your patch-set.


  in these guests the X server currently exits with "No
> devices detected", unless the user provides a config file with a Device
> section that identifies the card by BusID.
>
> Although this behavior matches the documentation precisely, it is
> inconvenient for virtual machines, where the X server is expected to start
> regardless of the (unpredictable) PCI B/D/F of the "virtio-gpu-pci"
> device.
>
> Modify xf86BusConfig() so that if no screen has been found, but there is
> at least one GPU screen, one of the GPU screens be promoted to the single
> normal screen. (This change should be either unnoticeable on physical
> platforms, or beneficial even.) For this, call the
> xf86PromoteLastGPUScreenToLastScreen() helper function added in the last
> patch.
>
> Update the manual page accordingly. The documentation remains
> intentionally vague on what GPU screen gets promoted under the above
> circumstances. The statements about multi-head setups, and about
> single-head setups using the primary graphics card, are preserved intact.
>
> Cc: Adam Jackson <ajax at redhat.com>
> Cc: Dave Airlie <airlied at redhat.com>
> Cc: Keith Packard <keithp at keithp.com>
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  hw/xfree86/common/xf86Bus.c  | 19 ++++++++++++++++---
>  hw/xfree86/man/xorg.conf.man | 21 ++++++++++++++++-----
>  2 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86Bus.c b/hw/xfree86/common/xf86Bus.c
> index dc83e2dde099..572d91fd223b 100644
> --- a/hw/xfree86/common/xf86Bus.c
> +++ b/hw/xfree86/common/xf86Bus.c
> @@ -127,10 +127,23 @@ xf86BusConfig(void)
>          xf86CallDriverProbe(xf86DriverList[i], FALSE);
>      }
>
> -    /* If nothing was detected, return now */
>      if (xf86NumScreens == 0) {
> -        xf86Msg(X_ERROR, "No devices detected.\n");
> -        return FALSE;
> +        if (xf86NumGPUScreens == 0) {
> +            /* If nothing was detected, return now */
> +            xf86Msg(X_ERROR, "No devices detected.\n");
> +            return FALSE;
> +        }
> +
> +        /*
> +         * If no screen was detected, but there is at least one GPU screen,
> +         * promote one of the latter to be the single screen. Note that a
> +         * graphics card matched by a Device.BusID (i.e., from the config file)
> +         * is added as a screen, not as a GPU screen; so if that happens, we
> +         * don't reach this code.
> +         */
> +        xf86PromoteLastGPUScreenToLastScreen();
> +        xf86Msg(X_WARNING, "Use Device.BusID to select a specific secondary "
> +                "graphics card.\n");
>      }
>
>      xf86VGAarbiterInit();
> diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
> index 94b199e6bac4..36f04febf2f4 100644
> --- a/hw/xfree86/man/xorg.conf.man
> +++ b/hw/xfree86/man/xorg.conf.man
> @@ -1353,11 +1353,22 @@ the
>  string has the form
>  .BI PCI: bus : device : function
>  (e.g., \(lqPCI:1:0:0\(rq might be appropriate for an AGP card).
> -This field is usually optional in single-head configurations when using
> -the primary graphics card.
> -In multi-head configurations, or when using a secondary graphics card in a
> -single-head configuration, this entry is mandatory.
> -Its main purpose is to make an unambiguous connection between the device
> +In multi-head configurations, this entry is mandatory.
> +It is usually optional in single-head configurations when using the primary
> +graphics card.
> +When using a secondary graphics card in a single-head configuration,
> +.B BusID
> +is recommended; if no graphics card can be identified as primary, nor is any
> +card selected by at least one
> +.B Device
> +section through the
> +.B BusID
> +field, then an unspecified secondary card (if any) is promoted to primary
> +status.
> +The main purpose of
> +.B BusID
> +is to make an unambiguous connection between the
> +.B Device
>  section and the hardware it is representing.
>  This information can usually be found by running the pciaccess tool
>  scanpci.
>