Re: [PATCH] Establish a preferred default of 1024x768 correctly.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]