[Spice-devel] Fwd: [qxl-win PATCH 1/2] Add a driver escape to pass monitor positions to Spice Server

Sandy Stutsman sstutsma at redhat.com
Mon Mar 23 07:44:34 PDT 2015


I've found that if the user uses the Windows "Screen Resolution" applet to configure the monitor positions, they all ultimately get positioned at 0.

For windows, there is a qxl driver instance per-monitor, hence no notion of multi-monitors and no handling of positions.  Without the escape, the driver doesn't have any position information. This results in the spice server sending monitor_config messages with 0,0 as the position, which the client then sends back to the guest. 

----- Original Message -----
From: "Marc-André Lureau" <marcandre.lureau at gmail.com>
To: "Sandy Stutsman" <sstutsma at redhat.com>
Cc: spice-devel at freedesktop.org
Sent: Monday, March 23, 2015 8:06:36 AM
Subject: Re: [Spice-devel] Fwd: [qxl-win PATCH 1/2] Add a driver escape to pass monitor positions to Spice Server

Hi Sandy,

Your patches didn't get through the first time, I would recommend you
re-git send-email all. As it is, it's unpleasant to extract or review
the patches. Furthermore, some are missing (2/2 of this for example).

On Sat, Mar 21, 2015 at 12:08 AM, Sandy Stutsman <sstutsma at redhat.com> wrote:
>
>
> ----- Forwarded Message -----
> From: sstutsma at redhat.com
> To: spice-devel at lists.freedesktop.org
> Cc: "Sandy Stutsman" <sstutsma at redhat.com>
> Sent: Wednesday, March 11, 2015 1:35:33 PM
> Subject: [qxl-win PATCH 1/2] Add a driver escape to pass monitor positions to Spice Server
>
> From: Sandy Stutsman <sstutsma at redhat.com>
>
> Add an escape handler to format monitor position/size  into a spice
> server monitors_config message and send it to qemu via async_io.
> Add code to allocate a monitors_config message from vram memory and set
> its physical address in the device's ram_header.

So the driver alone doesn't know its own position, and you need an
escape message for that?

What does this solve: the client rearanging monitors order because it
has them all at 0,0?

> ---
>  xddm/display/driver.c     | 35 +++++++++++++++++++++++++++++++++++
>  xddm/display/qxldd.h      |  5 +++++
>  xddm/display/res.c        | 16 +++++++++++++++-
>  xddm/include/qxl_driver.h |  6 +++++-
>  xddm/miniport/qxl.c       |  5 ++++-
>  5 files changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/xddm/display/driver.c b/xddm/display/driver.c
> index aa4fe42..56ea98b 100644
> --- a/xddm/display/driver.c
> +++ b/xddm/display/driver.c
> @@ -253,6 +253,27 @@ BOOL DrvEnableDriver(ULONG engine_version, ULONG enable_data_size, PDRVENABLEDAT
>      return TRUE;
>  }
>
> +static void SetMonitorConfig(PDev *pdev, QXLEscapeMonitorConfig * esc_config)
> +{
> +    QXLMonitorsConfig   *monitorConfig;
> +    QXLHead             *heads;
> +
> +
> +    pdev->monitor_config->count = 1;
> +    pdev->monitor_config->max_allowed = 1;
> +
> +    heads = &pdev->monitor_config->heads[0];
> +    heads->id = 0;
> +    heads->surface_id = 0;
> +    heads->x = esc_config->xpos;
> +    heads->y = esc_config->ypos;
> +    heads->width = esc_config->xres;
> +    heads->height = esc_config->yres;
> +    DEBUG_PRINT((pdev, 0, "%s Monitor %d (%d, %d) (%d x %d)\n",
> +        __FUNCTION__, pdev->dev_id, heads->x, heads->y, heads->width, heads->height));
> +    async_io(pdev, ASYNCABLE_MONITOR_CONFIG, 0);
> +}
> +
>  ULONG DrvEscape(SURFOBJ *pso, ULONG iEsc, ULONG cjIn, PVOID pvIn,
>                  ULONG cjOut, PVOID pvOut)
>  {
> @@ -275,6 +296,16 @@ ULONG DrvEscape(SURFOBJ *pso, ULONG iEsc, ULONG cjIn, PVOID pvIn,
>          RetVal = 1;
>          break;
>      }
> +    case QXL_ESCAPE_MONITOR_CONFIG: {
> +        ULONG length;
> +        DEBUG_PRINT((pdev, 1, "%s - 0x%p \n", __FUNCTION__, pdev));
> +        if (pdev == NULL || cjIn != sizeof(QXLEscapeMonitorConfig))
> +            break;
> +
> +        SetMonitorConfig(pdev, (QXLEscapeMonitorConfig * )pvIn);
> +        RetVal = 1;
> +        break;
> +    }
>      default:
>          DEBUG_PRINT((NULL, 1, "%s: unhandled escape code %d\n", __FUNCTION__, iEsc));
>          RetVal = 0;
> @@ -774,6 +805,9 @@ static BOOL PrepareHardware(PDev *pdev)
>      pdev->asyncable[ASYNCABLE_FLUSH_SURFACES][ASYNC] = dev_info.flush_surfaces_async_port;
>      pdev->asyncable[ASYNCABLE_FLUSH_SURFACES][SYNC] = NULL;
>
> +    pdev->asyncable[ASYNCABLE_MONITOR_CONFIG][ASYNC] = dev_info.monitors_config_port;
> +    pdev->asyncable[ASYNCABLE_MONITOR_CONFIG][SYNC] = NULL;
> +
>      pdev->display_event = dev_info.display_event;
>      pdev->cursor_event = dev_info.cursor_event;
>      pdev->sleep_event = dev_info.sleep_event;
> @@ -847,6 +881,7 @@ static BOOL PrepareHardware(PDev *pdev)
>      DEBUG_PRINT((pdev, 1, "%s: create_non_primary_surfaces = %d\n", __FUNCTION__,
>                   pdev->create_non_primary_surfaces));
>
> +    pdev->monitor_config_pa = dev_info.monitors_config;
>      CreateVRamSlot(pdev);
>
>      DEBUG_PRINT((NULL, 1, "%s: 0x%lx exit: 0x%lx %ul\n", __FUNCTION__, pdev,
> diff --git a/xddm/display/qxldd.h b/xddm/display/qxldd.h
> index a0b2eab..99e5c85 100644
> --- a/xddm/display/qxldd.h
> +++ b/xddm/display/qxldd.h
> @@ -180,6 +180,7 @@ typedef enum {
>      ASYNCABLE_DESTROY_SURFACE,
>      ASYNCABLE_DESTROY_ALL_SURFACES,
>      ASYNCABLE_FLUSH_SURFACES,
> +    ASYNCABLE_MONITOR_CONFIG,
>
>      ASYNCABLE_COUNT
>  } asyncable_t;
> @@ -349,6 +350,9 @@ typedef struct PDev {
>
>      UCHAR  pci_revision;
>
> +    QXLMonitorsConfig * monitor_config;
> +    QXLPHYSICAL * monitor_config_pa;
> +
>  #ifdef DBG
>      int num_free_pages;
>      int num_outputs;
> @@ -372,6 +376,7 @@ void DebugPrintV(PDev *pdev, const char *message, va_list ap);
>  void DebugPrint(PDev *pdev, int level, const char *message, ...);
>
>  void InitResources(PDev *pdev);
> +void InitMonitorConfig(PDev * pdev);
>  void ClearResources(PDev *pdev);
>
>  #ifdef CALL_TEST
> diff --git a/xddm/display/res.c b/xddm/display/res.c
> index bfb3571..3ea1760 100644
> --- a/xddm/display/res.c
> +++ b/xddm/display/res.c
> @@ -548,12 +548,26 @@ void ClearResources(PDev *pdev)
>      }
>  }
>
> +/*
> + * Tell the spice server where to look for updates to the monitor configuration.
> + */
> +void InitMonitorConfig(PDev *pdev)
> +{
> +    size_t monitor_config_size   = sizeof(QXLMonitorsConfig) + sizeof(QXLHead);
> +
> +    pdev->monitor_config      = AllocMem(pdev, MSPACE_TYPE_DEVRAM, monitor_config_size);
> +    RtlZeroMemory(pdev->monitor_config, monitor_config_size);
> +
> +    *pdev->monitor_config_pa  = PA(pdev, pdev->monitor_config, pdev->main_mem_slot);
> +}
> +
>  void InitResources(PDev *pdev)
>  {
>      DEBUG_PRINT((pdev, 3, "%s: entry\n", __FUNCTION__));
>
>      InitSurfaces(pdev);
>      InitDeviceMemoryResources(pdev);
> +    InitMonitorConfig(pdev);
>
>      pdev->update_id = *pdev->dev_update_id;
>
> @@ -2005,7 +2019,7 @@ static BOOL CacheSizeTest(PDev *pdev, SURFOBJ *surf)
>  {
>      BOOL ret = (UINT32)surf->sizlBitmap.cx * surf->sizlBitmap.cy <= pdev->max_bitmap_size;
>      if (!ret) {
> -        DEBUG_PRINT((pdev, 1, "%s: cache size test failed x %d y %d max\n",
> +        DEBUG_PRINT((pdev, 1, "%s: cache size test failed x %d y %d max %d\n",
>                       __FUNCTION__,
>                       surf->sizlBitmap.cx,
>                       surf->sizlBitmap.cy,
> diff --git a/xddm/include/qxl_driver.h b/xddm/include/qxl_driver.h
> index 677ee17..0ed73c4 100644
> --- a/xddm/include/qxl_driver.h
> +++ b/xddm/include/qxl_driver.h
> @@ -32,7 +32,8 @@
>  enum {
>      FIRST_AVIL_IOCTL_FUNC = 0x800,
>      QXL_GET_INFO_FUNC = FIRST_AVIL_IOCTL_FUNC,
> -    QXL_SET_CUSTOM_DISPLAY
> +    QXL_SET_CUSTOM_DISPLAY,
> +    QXL_SET_MONITOR_CONFIG
>  };
>
>  #define IOCTL_QXL_GET_INFO \
> @@ -109,6 +110,7 @@ typedef struct QXLDriverInfo {
>      PUCHAR memslot_add_port;
>      PUCHAR memslot_del_port;
>      PUCHAR destroy_all_surfaces_port;
> +    PUCHAR monitors_config_port;
>
>      UCHAR  pci_revision;
>
> @@ -121,6 +123,8 @@ typedef struct QXLDriverInfo {
>      UINT64 fb_phys;
>
>      UINT8 create_non_primary_surfaces;
> +
> +    QXLPHYSICAL * monitors_config;
>  } QXLDriverInfo;
>
>  #endif
> diff --git a/xddm/miniport/qxl.c b/xddm/miniport/qxl.c
> index f5d6b48..4670893 100644
> --- a/xddm/miniport/qxl.c
> +++ b/xddm/miniport/qxl.c
> @@ -1246,6 +1246,7 @@ BOOLEAN StartIO(PVOID dev_extension, PVIDEO_REQUEST_PACKET packet)
>              driver_info->destroy_primary_port = dev_ext->io_port + QXL_IO_DESTROY_PRIMARY;
>              driver_info->memslot_add_port = dev_ext->io_port + QXL_IO_MEMSLOT_ADD;
>              driver_info->memslot_del_port = dev_ext->io_port + QXL_IO_MEMSLOT_DEL;
> +            driver_info->monitors_config_port = dev_ext->io_port + QXL_IO_MONITORS_CONFIG_ASYNC;
>
>              driver_info->primary_surface_create = &dev_ext->ram_header->create_surface;
>
> @@ -1256,12 +1257,14 @@ BOOLEAN StartIO(PVOID dev_extension, PVIDEO_REQUEST_PACKET packet)
>              driver_info->dev_id = dev_ext->rom->id;
>
>              driver_info->create_non_primary_surfaces = check_non_primary_surfaces_registry_key(dev_ext);
> +
> +            driver_info->monitors_config = &dev_ext->ram_header->monitors_config;
>          }
>          break;
>
>      case IOCTL_QXL_SET_CUSTOM_DISPLAY: {
>              QXLEscapeSetCustomDisplay *custom_display;
> -            DEBUG_PRINT((dev_ext, 0, "%s: IOCTL_QXL_SET_CUSTOM_DISPLAY\n", __FUNCTION__));
> +            DEBUG_PRINT((dev_ext, 0, "%s: Device %d, IOCTL_QXL_SET_CUSTOM_DISPLAY\n", __FUNCTION__, dev_ext->rom->id));
>
>              if (packet->InputBufferLength < (packet->StatusBlock->Information =
>                                               sizeof(QXLEscapeSetCustomDisplay))) {
> --
> 2.1.0
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau


More information about the Spice-devel mailing list