crosvm: merge --gpu-display into --gpu in CLI.

Now that serde_keyvalue is capable of parsing vectors and
sub-structs, we are making the following pattern the official way
of providing display parameters:

--gpu backend=3d,displays=[[mode=windowed[800,600]],\
    [mode=borderless_full_screen]]

The reasons for this change are:
(1) We will likely use this syntax for other CLI options as well.
(2) We only support one single virtual GPU as of today, but
    technically we could support multiple. When we add that
    support, there is no ambiguity as to which display is connected
    to which GPU with this syntax.

For backwards compatibility, we will preserve the following ways to
provide display parameters:
(1) --gpu width=800,height=600 (which implies windowed mode)
(2) --gpu-display mode=windowed,width=800,height=600 \
        --gpu-display mode=borderless_full_screen

BUG=b:254284360
TEST=presubmit

Change-Id: I7f0078b7cf7554c1e93882afce47a4a2a3170a78
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3984545
Commit-Queue: Pujun Lun <lunpujun@google.com>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>

devices/src/virtio/gpu/parameters.rs

@@ -42,7 +42,7 @@ mod serde_context_mask {
 pub struct GpuParameters {
     #[serde(rename = "backend")]
     pub mode: GpuMode,
-    #[serde(skip)]
+    #[serde(rename = "displays")]
     pub display_params: Vec<DisplayParameters>,
     // `width` and `height` are supported for CLI backwards compatibility.
     #[serde(rename = "width")]

src/crosvm/cmdline.rs

@@ -489,7 +489,7 @@ pub enum GpuSubCommand {
 pub struct GpuAddDisplaysCommand {
     #[argh(option)]
     /// displays
-    pub gpu_display: Vec<vm_control::gpu::DisplayParameters>,
+    pub gpu_display: Vec<GpuDisplayParameters>,
 
     #[argh(positional, arg_name = "VM_SOCKET")]
     /// VM Socket path
@@ -1079,12 +1079,17 @@ pub struct RunCommand {
     /// Possible key values:
     ///     backend=(2d|virglrenderer|gfxstream) - Which backend to
     ///        use for virtio-gpu (determining rendering protocol)
+    ///     displays=[[GpuDisplayParameters]] - The list of virtual
+    ///         displays to create. See the possible key values for
+    ///         GpuDisplayParameters in the section below.
     ///     context-types=LIST - The list of supported context
     ///       types, separated by ':' (default: no contexts enabled)
     ///     width=INT - The width of the virtual display connected
     ///        to the virtio-gpu.
+    ///        Deprecated - use `displays` instead.
     ///     height=INT - The height of the virtual display
     ///        connected to the virtio-gpu.
+    ///        Deprecated - use `displays` instead.
     ///     egl[=true|=false] - If the backend should use a EGL
     ///        context for rendering.
     ///     glx[=true|=false] - If the backend should use a GLX
@@ -1103,15 +1108,8 @@ pub struct RunCommand {
     ///     cache-size=SIZE - The maximum size of the shader cache.
     ///     pci-bar-size=SIZE - The size for the PCI BAR in bytes
     ///        (default 8gb).
-    pub gpu: Vec<FixedGpuParameters>,
-
-    #[cfg(feature = "gpu")]
-    #[argh(option)]
-    #[serde(skip)] // TODO(b/255223604)
-    #[merge(strategy = append)]
-    /// (EXPERIMENTAL) Comma separated key=value pairs for setting
-    /// up a display on the virtio-gpu device
-    /// Possible key values:
+    ///
+    /// Possible key values for GpuDisplayParameters:
     ///     mode=(borderless_full_screen|windowed[width,height]) -
     ///        Whether to show the window on the host in full
     ///        screen or windowed mode. If not specified, windowed
@@ -1130,6 +1128,15 @@ pub struct RunCommand {
     ///     vertical-dpi=INT - The vertical DPI of the display
     ///        (default: 320)
     ///        Deprecated - use `dpi` instead.
+    pub gpu: Vec<FixedGpuParameters>,
+
+    #[cfg(feature = "gpu")]
+    #[argh(option)]
+    #[serde(skip)] // TODO(b/255223604). Deprecated - use `gpu` instead.
+    #[merge(strategy = append)]
+    /// (EXPERIMENTAL) Comma separated key=value pairs for setting
+    /// up a display on the virtio-gpu device. See comments for `gpu`
+    /// for possible key values of GpuDisplayParameters.
     pub gpu_display: Vec<FixedGpuDisplayParameters>,
 
     #[cfg(all(unix, feature = "gpu", feature = "virgl_renderer_next"))]

src/crosvm/gpu_config.rs

@@ -659,19 +659,71 @@ mod tests {
     }
 
     #[test]
-    fn parse_gpu_options_and_gpu_display_options_valid() {
-        const WIDTH: u32 = 1720;
-        const HEIGHT: u32 = 1800;
-        const EXPECTED_DISPLAY_MODE: GpuDisplayMode = GpuDisplayMode::Windowed(WIDTH, HEIGHT);
+    fn parse_gpu_options_single_display() {
+        {
+            let gpu_params = parse_gpu_options("displays=[[mode=windowed[800,600]]]").unwrap();
+            assert_eq!(gpu_params.display_params.len(), 1);
+            assert_eq!(
+                gpu_params.display_params[0].mode,
+                GpuDisplayMode::Windowed(800, 600)
+            );
+        }
+
+        #[cfg(windows)]
+        {
+            let gpu_params = parse_gpu_options("displays=[[mode=borderless_full_screen]]").unwrap();
+            assert_eq!(gpu_params.display_params.len(), 1);
+            assert!(matches!(
+                gpu_params.display_params[0].mode,
+                GpuDisplayMode::BorderlessFullScreen(_)
+            ));
+        }
+    }
+
+    #[test]
+    fn parse_gpu_options_multi_display() {
+        {
+            let gpu_params =
+                parse_gpu_options("displays=[[mode=windowed[500,600]],[mode=windowed[700,800]]]")
+                    .unwrap();
+            assert_eq!(gpu_params.display_params.len(), 2);
+            assert_eq!(
+                gpu_params.display_params[0].mode,
+                GpuDisplayMode::Windowed(500, 600)
+            );
+            assert_eq!(
+                gpu_params.display_params[1].mode,
+                GpuDisplayMode::Windowed(700, 800)
+            );
+        }
+
+        #[cfg(windows)]
+        {
+            let gpu_params = parse_gpu_options(
+                "displays=[[mode=windowed[800,600]],[mode=borderless_full_screen]]",
+            )
+            .unwrap();
+            assert_eq!(gpu_params.display_params.len(), 2);
+            assert_eq!(
+                gpu_params.display_params[0].mode,
+                GpuDisplayMode::Windowed(800, 600)
+            );
+            assert!(matches!(
+                gpu_params.display_params[1].mode,
+                GpuDisplayMode::BorderlessFullScreen(_)
+            ));
+        }
+    }
+
+    #[test]
+    fn parse_gpu_options_single_display_compat() {
         const BACKEND: &str = get_backend_name();
 
         let config: Config = crate::crosvm::cmdline::RunCommand::from_args(
             &[],
             &[
                 "--gpu",
-                format!("backend={}", BACKEND).as_str(),
-                "--gpu-display",
-                format!("windowed[{},{}]", WIDTH, HEIGHT).as_str(),
+                format!("backend={},width=500,height=600", BACKEND,).as_str(),
                 "/dev/null",
             ],
         )
@@ -682,14 +734,18 @@ mod tests {
         let gpu_params = config.gpu_parameters.unwrap();
 
         assert_eq!(gpu_params.display_params.len(), 1);
-        assert_eq!(gpu_params.display_params[0].mode, EXPECTED_DISPLAY_MODE);
+        assert_eq!(
+            gpu_params.display_params[0].mode,
+            GpuDisplayMode::Windowed(500, 600)
+        );
 
-        // `width` and `height` in GPU options are supported for CLI backward compatibility.
         let config: Config = crate::crosvm::cmdline::RunCommand::from_args(
             &[],
             &[
                 "--gpu",
-                format!("backend={},width={},height={}", BACKEND, WIDTH, HEIGHT).as_str(),
+                format!("backend={}", BACKEND,).as_str(),
+                "--gpu-display",
+                "mode=windowed[700,800]",
                 "/dev/null",
             ],
         )
@@ -700,7 +756,10 @@ mod tests {
         let gpu_params = config.gpu_parameters.unwrap();
 
         assert_eq!(gpu_params.display_params.len(), 1);
-        assert_eq!(gpu_params.display_params[0].mode, EXPECTED_DISPLAY_MODE);
+        assert_eq!(
+            gpu_params.display_params[0].mode,
+            GpuDisplayMode::Windowed(700, 800)
+        );
     }
 
     #[cfg(unix)]
@@ -711,9 +770,13 @@ mod tests {
                 &[],
                 &[
                     "--gpu",
-                    format!("backend={},width=500,height=600", get_backend_name()).as_str(),
+                    format!(
+                        "backend={},width=500,height=600,displays=[[mode=windowed[700,800]]]",
+                        get_backend_name()
+                    )
+                    .as_str(),
                     "--gpu-display",
-                    "mode=windowed[700,800]",
+                    "mode=windowed[900,1000]",
                     "/dev/null",
                 ],
             )
@@ -723,14 +786,18 @@ mod tests {
 
             let gpu_params = config.gpu_parameters.unwrap();
 
-            assert_eq!(gpu_params.display_params.len(), 2);
+            assert_eq!(gpu_params.display_params.len(), 3);
             assert_eq!(
-                gpu_params.display_params[0].get_virtual_display_size(),
-                (500, 600),
+                gpu_params.display_params[0].mode,
+                GpuDisplayMode::Windowed(700, 800)
             );
             assert_eq!(
-                gpu_params.display_params[1].get_virtual_display_size(),
-                (700, 800),
+                gpu_params.display_params[1].mode,
+                GpuDisplayMode::Windowed(500, 600)
+            );
+            assert_eq!(
+                gpu_params.display_params[2].mode,
+                GpuDisplayMode::Windowed(900, 1000)
             );
         }
     }
@@ -763,6 +830,30 @@ mod tests {
         )
         .unwrap();
         assert!(Config::try_from(command).is_err());
+
+        let command = crate::crosvm::cmdline::RunCommand::from_args(
+            &[],
+            &[
+                "--gpu",
+                "displays=[[mode=windowed[1280,720]]]",
+                "--gpu-display",
+                "mode=borderless_full_screen",
+                "/dev/null",
+            ],
+        )
+        .unwrap();
+        assert!(Config::try_from(command).is_err());
+
+        let command = crate::crosvm::cmdline::RunCommand::from_args(
+            &[],
+            &[
+                "--gpu",
+                "displays=[[mode=windowed[500,600]],[mode=windowed[700,800]]]",
+                "/dev/null",
+            ],
+        )
+        .unwrap();
+        assert!(Config::try_from(command).is_err());
     }
 
     #[test]