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@xxxxxxxxxxxxxxx> --- 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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel