On Wed, Sep 16, 2015 at 03:27:49PM -0400, Sandy Stutsman wrote:
>
> Modes are returned in a buffer from qemu with IDs in increasing but not
> necessarily consecutive order. (Some modes may be omitted if they don't fit
> into the framebuffer.) Two custom modes are appended to the local buffer to
> support arbitrary resolutions. The first custom id must be set to +1 of the
> last id in the mode buffer, not to the last index +1 of the buffer.
>
> E.G. these are the last entries in the mode buffer returned from QEMU:
> Index Mode id (unfortunately named ModeIndex in the struct)
> ...
> 124 128
> 125 130
> 126 132
> 127 134 (last mode returned from QEMU)
>
> Custom Modes:
> 128 135 (Not 128!)
> 129 136
>
> If the first custom mode id is set to 128 (the number of modes returned
> from QEMU), it will duplicate the id for the mode at index 124. That could
> cause the FindMode function which searches the mode list by mode id
> to return the wrong mode information.
>
> The custom mode id's are now set to be greater than the last mode id
> returned from QEMU.
> ---
> xddm/miniport/qxl.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/xddm/miniport/qxl.c b/xddm/miniport/qxl.c
> index e0fb87f..22537e2 100644
> --- a/xddm/miniport/qxl.c
> +++ b/xddm/miniport/qxl.c
> @@ -596,6 +596,7 @@ VP_STATUS InitModes(QXLExtension *dev)
> ULONG n_modes;
> ULONG i;
> VP_STATUS error;
> + ULONG custom_mode_id = 0;
>
> PAGED_CODE();
> DEBUG_PRINT((dev, 0, "%s\n", __FUNCTION__));
> @@ -632,6 +633,11 @@ VP_STATUS InitModes(QXLExtension *dev)
> return error;
> }
> }
> + /* Mode ids are increasing in the buffer but may not be consecutive,
> + * so set the first custom id to be 1 larger than the last id in the buffer.
> + * (ModeIndex isn't really an index, it is an ID).
> + */
> + custom_mode_id = modes->modes[modes->n_modes-1].id + 1;
>
> /* 2 dummy modes for custom display resolution */
> /* This is necessary to bypass Windows mode index check, that
> @@ -640,7 +646,7 @@ VP_STATUS InitModes(QXLExtension *dev)
>
> for (i = dev->custom_mode; i <= dev->custom_mode + 1; ++i) {
> memcpy(&modes_info[i], &modes_info[0], sizeof(VIDEO_MODE_INFORMATION));
> - modes_info[i].ModeIndex = i;
> + modes_info[i].ModeIndex = custom_mode_id++;
> }
>
> dev->n_modes = n_modes;
> @@ -1002,6 +1008,7 @@ static VP_STATUS SetCustomDisplay(QXLExtension *dev_ext, QXLEscapeSetCustomDispl
> uint32_t xres = custom_display->xres;
> uint32_t yres = custom_display->yres;
> uint32_t bpp = custom_display->bpp;
> + PVIDEO_MODE_INFORMATION pMode;
>
> /* alternate custom mode index */
> if (dev_ext->custom_mode == (dev_ext->n_modes - 1))
> @@ -1014,11 +1021,11 @@ static VP_STATUS SetCustomDisplay(QXLExtension *dev_ext, QXLEscapeSetCustomDispl
> __FUNCTION__, xres, yres, bpp, dev_ext->rom->surface0_area_size));
> return ERROR_NOT_ENOUGH_MEMORY;
> }
> -
> - ret = FillVidModeInfo(&dev_ext->modes[dev_ext->custom_mode],
> + pMode = &dev_ext->modes[dev_ext->custom_mode];
> + ret = FillVidModeInfo(pMode,
> custom_display->xres, custom_display->yres,
> custom_display->bpp,
> - dev_ext->custom_mode);
> + pMode->ModeIndex);
This looks good to me, ACK.
I'd add a commit on top of that getting rid of
the last argument to FillVidModeInfo as all that function does with this
argument is set pMode->ModeIndex to be equal to its value.
Christophe
Attachment:
pgpJgftHELjqz.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel