[Spice-devel] [PATCH] Establish a preferred default of 1024x768 correctly.
Hans de Goede
hdegoede at redhat.com
Thu Jan 24 03:37:13 PST 2013
Hi,
Overall comments:
1) I've run several tests with this, and this looks good to go.
2) As Alon said, please split of the #if 0 blocks removal, I agree they can
be removed after this patch, but lets do so in a follow-up patch
3) I don't like the hardcoded 1024x768, yes I know it was there before, but
while at it can you please add:
#define DEFAULT_WIDTH 1024
#define DEFAULT_HEIGHT 768
To the top of qxl_driver.c and use those in the check to set the preferred
flag?
Also one small white spice issues, see comments inline.
I believe that with the above items fixed this patch can be pushed.
On 01/23/2013 11:40 PM, Jeremy White wrote:
> This fixes a bug with x-spice where you could not specify
> a default mode in an xorg.conf modeline that was greater
> than 1024x768. This also eliminates (and partially
> reverts) patch c1b537fc.
>
> It may also fix bug 894421, where gnome modes flicker
> and work poorly.
>
> Signed-off-by: Jeremy White <jwhite at codeweavers.com>
> ---
> src/qxl_driver.c | 90 ++++++++++++------------------------------------------
> 1 file changed, 20 insertions(+), 70 deletions(-)
>
> diff --git a/src/qxl_driver.c b/src/qxl_driver.c
> index e1893dc..546be6b 100644
> --- a/src/qxl_driver.c
> +++ b/src/qxl_driver.c
> @@ -951,13 +951,6 @@ qxl_resize_primary_to_virtual (qxl_screen_t *qxl)
> {
> ScreenPtr pScreen;
> long new_surface0_size;
> -
> - if ((qxl->primary_mode.x_res == qxl->virtual_x &&
> - qxl->primary_mode.y_res == qxl->virtual_y) &&
> - qxl->device_primary == QXL_DEVICE_PRIMARY_CREATED)
> - {
> - return TRUE; /* empty Success */
> - }
>
> ErrorF ("resizing primary to %dx%d\n", qxl->virtual_x, qxl->virtual_y);
>
> @@ -1780,15 +1773,7 @@ qxl_screen_init (SCREEN_INIT_ARGS_DECL)
> goto out;
> if (!miSetPixmapDepths ())
> goto out;
> - pScrn->displayWidth = pScrn->virtualX;
>
> -#if 0
> - ErrorF ("allocated %d x %d %p\n", pScrn->virtualX, pScrn->virtualY, qxl->fb);
> -#endif
> -
> - pScrn->virtualX = pScrn->currentMode->HDisplay;
> - pScrn->virtualY = pScrn->currentMode->VDisplay;
> -
> /* Set up resources */
> qxl_reset_and_create_mem_slots (qxl);
> ErrorF ("done reset\n");
> @@ -2083,15 +2068,6 @@ qxl_output_get_modes (xf86OutputPtr output)
> qxl_output_private *qxl_output = output->driver_private;
> DisplayModePtr modes = xf86DuplicateModes (qxl_output->qxl->pScrn, qxl_output->qxl->x_modes);
>
> - if (output &&
> - output->crtc && output->crtc->enabled)
> - {
> - DisplayModePtr crtc_mode = &output->crtc->mode;
> - crtc_mode = screen_create_mode (qxl_output->qxl->pScrn, crtc_mode->HDisplay, crtc_mode->VDisplay, M_T_PREFERRED);
> - output->crtc->mode = *crtc_mode;
> - modes = xf86ModesAdd (modes, crtc_mode);
> - }
> -
> /* xf86ProbeOutputModes owns this memory */
> return modes;
> }
> @@ -2322,12 +2298,6 @@ qxl_init_randr (ScrnInfoPtr pScrn, qxl_screen_t *qxl)
> qxl_crtc->output = output;
> }
>
> - qxl->virtual_x = 1024;
> - qxl->virtual_y = 768;
> -
> - pScrn->display->virtualX = qxl->virtual_x;
> - pScrn->display->virtualY = qxl->virtual_y;
> -
> xf86InitialConfiguration (pScrn, TRUE);
> /* all crtcs are enabled here, but their mode is 0,
> resulting monitor config empty atm */
> @@ -2339,6 +2309,7 @@ qxl_initialize_x_modes (qxl_screen_t *qxl, ScrnInfoPtr pScrn,
> {
> int i;
> int size;
> + int preferred_flag;
>
> *max_x = *max_y = 0;
> /* Create a list of modes used by the qxl_output_get_modes */
> @@ -2353,15 +2324,24 @@ qxl_initialize_x_modes (qxl_screen_t *qxl, ScrnInfoPtr pScrn,
> qxl->modes[i].x_res, qxl->modes[i].y_res);
> continue;
> }
> -
> +
You're changing whitespace here. Either don't change it all, or if you do, nuke
it all (other then the newline). What you do now makes git am complain, and
rightfully so.
> + if (qxl->modes[i].x_res == 1024 && qxl->modes[i].y_res == 768)
> + preferred_flag = M_T_PREFERRED;
> + else
> + preferred_flag = 0;
> +
> qxl_add_mode (qxl, pScrn, qxl->modes[i].x_res, qxl->modes[i].y_res,
> - M_T_DRIVER);
> + M_T_DRIVER | preferred_flag);
> if (qxl->modes[i].x_res > *max_x)
> *max_x = qxl->modes[i].x_res;
> if (qxl->modes[i].y_res > *max_y)
> *max_y = qxl->modes[i].y_res;
> }
> }
> +
> + pScrn->virtualX = pScrn->displayWidth = *max_x;
> + pScrn->virtualY = *max_y;
> + pScrn->virtualFrom = X_PROBED;
> }
>
> static Bool
> @@ -2489,51 +2469,21 @@ qxl_pre_init (ScrnInfoPtr pScrn, int flags)
> pScrn->monitor->vrefresh[0].hi = 75;
> pScrn->monitor->nVrefresh = 1;
> }
> -
> +
> + /* Note that we replace the 'normal' xf86ValidateModes call,
> + so this function is obligated to set the same values as
> + that call normally does. */
> qxl_initialize_x_modes (qxl, pScrn, &max_x, &max_y);
>
> -#if 0
> - if (pScrn->display->virtualX == 0 && pScrn->display->virtualY == 0)
> - {
> - /* It is possible for the largest x + largest y size combined leading
> - to a virtual size which will not fit into the framebuffer when this
> - happens we prefer max width and make height as large as possible */
> - if (max_x * max_y * (pScrn->bitsPerPixel / 8) >
> - qxl->rom->surface0_area_size)
> - pScrn->display->virtualY = qxl->rom->surface0_area_size /
> - (max_x * (pScrn->bitsPerPixel / 8));
> - else
> - pScrn->display->virtualY = max_y;
> -
> - pScrn->display->virtualX = max_x;
> - }
> -
> - if (0 >= xf86ValidateModes (pScrn, pScrn->monitor->Modes,
> - pScrn->display->modes, clockRanges, linePitches,
> - 128, max_x, 128 * 4, 128, max_y,
> - pScrn->display->virtualX,
> - pScrn->display->virtualY,
> - 128 * 1024 * 1024, LOOKUP_BEST_REFRESH))
> - goto out;
> -#endif
> -
> CHECK_POINT ();
>
> xf86PruneDriverModes (pScrn);
>
> qxl_init_randr (pScrn, qxl);
> -#if 0
> - /* If no modes are specified in xorg.conf, default to 1024x768 */
> - if (pScrn->display->modes == NULL || pScrn->display->modes[0] == NULL)
> - for (mode = pScrn->modes; mode; mode = mode->next)
> - if (mode->HDisplay == 1024 && mode->VDisplay == 768)
> - {
> - pScrn->currentMode = mode;
> - break;
> - }
> -#endif
> -
> - //xf86PrintModes (pScrn);
> +
> + qxl->virtual_x = pScrn->display->virtualX;
> + qxl->virtual_y = pScrn->display->virtualY;
> +
> xf86SetDpi (pScrn, 0, 0);
>
> if (!xf86LoadSubModule (pScrn, "fb")
>
Regards,
Hans
More information about the Spice-devel
mailing list