Hey.
On 9/17/2015 11:06 AM, Uri Lublin wrote:
> On 09/17/2015 02:40 PM, Christophe Fergeau wrote:
>> 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.
> This patch is good, but I wonder if the problem is not with qemu.
> If we change in qemu/hw/display/qxl.c:
> - modes->modes[n].id = cpu_to_le32(i);
> + modes->modes[n].id = cpu_to_le32(n);
>
> Does that solves the problem ?
>
> Thanks,
> Uri.
>
I bet it does and I can test it for the windows driver but I'd need help to verify it for linux :)
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel