[PATCH 1/6] Replace huge argument list in xf86CrtcSetModeTransform with struct

Keith Packard keithp at keithp.com
Sun Dec 5 14:22:47 PST 2010


xf86CrtcSetModeTransform was starting to get ridiculous with 6
arguments, this change has it take a single structure that contains
all of those values along with a set of flags that says which have
changed.

Signed-off-by: Keith Packard <keithp at keithp.com>
---
 hw/xfree86/modes/xf86Crtc.c    |  120 ++++++++++++++++++++++++----------------
 hw/xfree86/modes/xf86Crtc.h    |   33 ++++++++---
 hw/xfree86/modes/xf86RandR12.c |   29 ++++++----
 3 files changed, 116 insertions(+), 66 deletions(-)

diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index 74d91ed..5e9fc69 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
@@ -247,15 +247,14 @@ xf86CrtcSetScreenSubpixelOrder (ScreenPtr pScreen)
  * Sets the given video mode on the given crtc
  */
 Bool
-xf86CrtcSetModeTransform (xf86CrtcPtr crtc, DisplayModePtr mode, Rotation rotation,
-			  RRTransformPtr transform, int x, int y)
+xf86CrtcSet(xf86CrtcPtr crtc, xf86CrtcSetRec *set)
 {
     ScrnInfoPtr		scrn = crtc->scrn;
     xf86CrtcConfigPtr   xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
     int			i;
     Bool		ret = FALSE;
     Bool		didLock = FALSE;
-    DisplayModePtr	adjusted_mode;
+    DisplayModePtr	adjusted_mode = NULL;
     DisplayModeRec	saved_mode;
     int			saved_x, saved_y;
     Rotation		saved_rotation;
@@ -272,8 +271,9 @@ xf86CrtcSetModeTransform (xf86CrtcPtr crtc, DisplayModePtr mode, Rotation rotati
 	return TRUE;
     }
 
-    adjusted_mode = xf86DuplicateMode(mode);
-
+    /* See if nothing has changed */
+    if (!set->flags)
+	return TRUE;
 
     saved_mode = crtc->mode;
     saved_x = crtc->x;
@@ -288,21 +288,44 @@ xf86CrtcSetModeTransform (xf86CrtcPtr crtc, DisplayModePtr mode, Rotation rotati
     /* Update crtc values up front so the driver can rely on them for mode
      * setting.
      */
-    crtc->mode = *mode;
-    crtc->x = x;
-    crtc->y = y;
-    crtc->rotation = rotation;
-    if (transform) {
-	RRTransformCopy (&crtc->transform, transform);
-	crtc->transformPresent = TRUE;
-    } else
-	crtc->transformPresent = FALSE;
+    if (set->flags & XF86CrtcSetMode)
+	crtc->mode = *set->mode;
+    if (set->flags & XF86CrtcSetOrigin) {
+	crtc->x = set->x;
+	crtc->y = set->y;
+    }
+    if (set->flags & XF86CrtcSetRotation)
+	crtc->rotation = set->rotation;
+
+    if (set->flags & XF86CrtcSetTransform) {
+	if (set->transform) {
+	    RRTransformCopy (&crtc->transform, set->transform);
+	    crtc->transformPresent = TRUE;
+	} else
+	    crtc->transformPresent = FALSE;
+    }
+
+    if (crtc->funcs->set) {
+	ret = crtc->funcs->set(crtc, set->flags);
+	goto done;
+    }
+
+    if (set->flags == XF86CrtcSetOrigin && crtc->funcs->set_origin) {
+	ret = xf86CrtcRotate(crtc);
+	if (ret)
+	    crtc->funcs->set_origin(crtc, crtc->x, crtc->y);
+	goto done;
+    }
 
     if (crtc->funcs->set_mode_major) {
-	ret = crtc->funcs->set_mode_major(crtc, mode, rotation, x, y);
+	ret = crtc->funcs->set_mode_major(crtc, &crtc->mode,
+					  crtc->rotation,
+					  crtc->x, crtc->y);
 	goto done;
     }
 
+    adjusted_mode = xf86DuplicateMode(&crtc->mode);
+
     didLock = crtc->funcs->lock (crtc);
     /* Pass our mode to the outputs and the CRTC to give them a chance to
      * adjust it according to limitations or output properties, and also
@@ -314,12 +337,12 @@ xf86CrtcSetModeTransform (xf86CrtcPtr crtc, DisplayModePtr mode, Rotation rotati
 	if (output->crtc != crtc)
 	    continue;
 
-	if (!output->funcs->mode_fixup(output, mode, adjusted_mode)) {
+	if (!output->funcs->mode_fixup(output, &crtc->mode, adjusted_mode)) {
 	    goto done;
 	}
     }
 
-    if (!crtc->funcs->mode_fixup(crtc, mode, adjusted_mode)) {
+    if (!crtc->funcs->mode_fixup(crtc, &crtc->mode, adjusted_mode)) {
 	goto done;
     }
 
@@ -342,12 +365,12 @@ xf86CrtcSetModeTransform (xf86CrtcPtr crtc, DisplayModePtr mode, Rotation rotati
     /* Set up the DPLL and any output state that needs to adjust or depend
      * on the DPLL.
      */
-    crtc->funcs->mode_set(crtc, mode, adjusted_mode, crtc->x, crtc->y);
+    crtc->funcs->mode_set(crtc, &crtc->mode, adjusted_mode, crtc->x, crtc->y);
     for (i = 0; i < xf86_config->num_output; i++) 
     {
 	xf86OutputPtr output = xf86_config->output[i];
 	if (output->crtc == crtc)
-	    output->funcs->mode_set(output, mode, adjusted_mode);
+	    output->funcs->mode_set(output, &crtc->mode, adjusted_mode);
     }
 
     /* Only upload when needed, to avoid unneeded delays. */
@@ -383,8 +406,10 @@ done:
 	crtc->transformPresent = saved_transform_present;
     }
 
-    free(adjusted_mode->name);
-    free(adjusted_mode);
+    if (adjusted_mode) {
+	free(adjusted_mode->name);
+	free(adjusted_mode);
+    }
 
     if (didLock)
 	crtc->funcs->unlock (crtc);
@@ -393,35 +418,19 @@ done:
 }
 
 /**
- * Sets the given video mode on the given crtc, but without providing
- * a transform
- */
-Bool
-xf86CrtcSetMode (xf86CrtcPtr crtc, DisplayModePtr mode, Rotation rotation,
-		 int x, int y)
-{
-    return xf86CrtcSetModeTransform (crtc, mode, rotation, NULL, x, y);
-}
-
-/**
  * Pans the screen, does not change the mode
  */
 void
 xf86CrtcSetOrigin (xf86CrtcPtr crtc, int x, int y)
 {
-    ScrnInfoPtr scrn = crtc->scrn;
+    xf86CrtcSetRec	set;
 
-    crtc->x = x;
-    crtc->y = y;
-    if (crtc->funcs->set_origin) {
-	if (!xf86CrtcRotate (crtc))
-	    return;
-	crtc->funcs->set_origin (crtc, x, y);
-	if (scrn->ModeSet)
-	    scrn->ModeSet(scrn);
+    if (x != crtc->x || y != crtc->y) {
+	set.x = x;
+	set.y = y;
+	set.flags = XF86CrtcSetOrigin;
+	(void) xf86CrtcSet(crtc, &set);
     }
-    else
-	xf86CrtcSetMode (crtc, &crtc->mode, crtc->rotation, x, y);
 }
 
 /*
@@ -2619,6 +2628,7 @@ xf86SetDesiredModes (ScrnInfoPtr scrn)
     for (c = 0; c < config->num_crtc; c++)
     {
 	xf86OutputPtr	output = NULL;
+	xf86CrtcSetRec	set;
 	int		o;
 	RRTransformPtr	transform;
 
@@ -2662,8 +2672,15 @@ xf86SetDesiredModes (ScrnInfoPtr scrn)
 	    transform = &crtc->desiredTransform;
 	else
 	    transform = NULL;
-	if (!xf86CrtcSetModeTransform (crtc, &crtc->desiredMode, crtc->desiredRotation,
-				       transform, crtc->desiredX, crtc->desiredY))
+	set.mode = &crtc->desiredMode;
+	set.rotation = crtc->desiredRotation;
+	set.transform = transform;
+	set.x = crtc->desiredX;
+	set.y = crtc->desiredY;
+	set.flags = (XF86CrtcSetMode | XF86CrtcSetOutput |
+		     XF86CrtcSetOrigin | XF86CrtcSetTransform |
+		     XF86CrtcSetRotation);
+	if (!xf86CrtcSet(crtc, &set))
 	    return FALSE;
     }
 
@@ -2767,6 +2784,7 @@ xf86SetSingleMode (ScrnInfoPtr pScrn, DisplayModePtr desired, Rotation rotation)
 	xf86CrtcPtr	crtc = config->crtc[c];
 	DisplayModePtr	crtc_mode = NULL;
 	int		o;
+	xf86CrtcSetRec	set;
 
 	if (!crtc->enabled)
 	    continue;
@@ -2794,7 +2812,15 @@ xf86SetSingleMode (ScrnInfoPtr pScrn, DisplayModePtr desired, Rotation rotation)
 	    crtc->enabled = FALSE;
 	    continue;
 	}
-	if (!xf86CrtcSetModeTransform (crtc, crtc_mode, rotation, NULL, 0, 0))
+	set.mode = crtc_mode;
+	set.rotation = rotation;
+	set.transform = NULL;
+	set.x = 0;
+	set.y = 0;
+	set.flags = (XF86CrtcSetMode | XF86CrtcSetOutput |
+		     XF86CrtcSetOrigin | XF86CrtcSetTransform |
+		     XF86CrtcSetRotation);
+	if (!xf86CrtcSet (crtc, &set))
 	    ok = FALSE;
 	else
 	{
diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
index 68a968c..f43e0a7 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -72,6 +72,23 @@ typedef enum _xf86OutputStatus {
    XF86OutputStatusUnknown
 } xf86OutputStatus;
 
+typedef enum _xf86CrtcSetFlags {
+    XF86CrtcSetMode = 1,		/* mode */
+    XF86CrtcSetOutput = 2,		/* outputs */
+    XF86CrtcSetOrigin = 4,		/* x/y */
+    XF86CrtcSetTransform = 8,		/* transform */
+    XF86CrtcSetRotation = 16,		/* rotation */
+    XF86CrtcSetProperty = 32,		/* output property */
+} xf86CrtcSetFlags;
+
+typedef struct _xf86CrtcSet {
+    xf86CrtcSetFlags	flags;
+    DisplayModePtr	mode;
+    Rotation		rotation;
+    RRTransformPtr	transform;
+    int			x, y;
+} xf86CrtcSetRec;
+
 typedef struct _xf86CrtcFuncs {
    /**
     * Turns the crtc on/off, or sets intermediate power levels if available.
@@ -221,6 +238,12 @@ typedef struct _xf86CrtcFuncs {
     void
     (*set_origin)(xf86CrtcPtr crtc, int x, int y);
 
+    /**
+     * General mode setting entry point that does everything
+     */
+    Bool
+    (*set)(xf86CrtcPtr crtc, xf86CrtcSetFlags flags);
+
 } xf86CrtcFuncsRec, *xf86CrtcFuncsPtr;
 
 #define XF86_CRTC_VERSION 3
@@ -738,18 +761,12 @@ xf86CrtcCreate (ScrnInfoPtr		scrn,
 extern _X_EXPORT void
 xf86CrtcDestroy (xf86CrtcPtr		crtc);
 
-
 /**
- * Sets the given video mode on the given crtc
+ * Change a crtc configuration (modes, outputs, etc)
  */
 
 extern _X_EXPORT Bool
-xf86CrtcSetModeTransform (xf86CrtcPtr crtc, DisplayModePtr mode, Rotation rotation,
-			  RRTransformPtr transform, int x, int y);
-
-extern _X_EXPORT Bool
-xf86CrtcSetMode (xf86CrtcPtr crtc, DisplayModePtr mode, Rotation rotation,
-		 int x, int y);
+xf86CrtcSet (xf86CrtcPtr crtc, xf86CrtcSetRec *set);
 
 extern _X_EXPORT void
 xf86CrtcSetOrigin (xf86CrtcPtr crtc, int x, int y);
diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
index 2fe0c47..d60ee3c 100644
--- a/hw/xfree86/modes/xf86RandR12.c
+++ b/hw/xfree86/modes/xf86RandR12.c
@@ -1133,7 +1133,7 @@ xf86RandR12CrtcSet (ScreenPtr	    pScreen,
     xf86CrtcConfigPtr   config = XF86_CRTC_CONFIG_PTR(pScrn);
     xf86CrtcPtr		crtc = randr_crtc->devPrivate;
     RRTransformPtr	transform;
-    Bool		changed = FALSE;
+    xf86CrtcSetFlags	flags = 0;
     int			o, ro;
     xf86CrtcPtr		*save_crtcs;
     Bool		save_enabled = crtc->enabled;
@@ -1143,22 +1143,22 @@ xf86RandR12CrtcSet (ScreenPtr	    pScreen,
 
     save_crtcs = malloc(config->num_output * sizeof (xf86CrtcPtr));
     if ((randr_mode != NULL) != crtc->enabled)
-	changed = TRUE;
+	flags |= XF86CrtcSetMode;
     else if (randr_mode && !xf86RandRModeMatches (randr_mode, &crtc->mode))
-	changed = TRUE;
+	flags |= XF86CrtcSetMode;
     
     if (rotation != crtc->rotation)
-	changed = TRUE;
+	flags |= XF86CrtcSetRotation;
 
     transform = RRCrtcGetTransform (randr_crtc);
     if ((transform != NULL) != crtc->transformPresent)
-	changed = TRUE;
+	flags |= XF86CrtcSetTransform;
     else if (transform && memcmp (&transform->transform, &crtc->transform.transform,
 				  sizeof (transform->transform)) != 0)
-	changed = TRUE;
+	flags |= XF86CrtcSetTransform;
 
     if (x != crtc->x || y != crtc->y)
-	changed = TRUE;
+	flags |= XF86CrtcSetOrigin;
     for (o = 0; o < config->num_output; o++) 
     {
 	xf86OutputPtr  output = config->output[o];
@@ -1178,16 +1178,16 @@ xf86RandR12CrtcSet (ScreenPtr	    pScreen,
 	    }
 	if (new_crtc != output->crtc)
 	{
-	    changed = TRUE;
+	    flags |= XF86CrtcSetOutput;
 	    output->crtc = new_crtc;
 	}
     }
     for (ro = 0; ro < num_randr_outputs; ro++) 
         if (randr_outputs[ro]->pendingProperties)
-	    changed = TRUE;
+	    flags |= XF86CrtcSetProperty;
 
     /* XXX need device-independent mode setting code through an API */
-    if (changed)
+    if (flags)
     {
 	crtc->enabled = randr_mode != NULL;
 
@@ -1195,9 +1195,16 @@ xf86RandR12CrtcSet (ScreenPtr	    pScreen,
 	{
 	    DisplayModeRec  mode;
 	    RRTransformPtr  transform = RRCrtcGetTransform (randr_crtc);
+	    xf86CrtcSetRec  set;
 
 	    xf86RandRModeConvert (pScrn, randr_mode, &mode);
-	    if (!xf86CrtcSetModeTransform (crtc, &mode, rotation, transform, x, y))
+	    set.mode = &mode;
+	    set.rotation = rotation;
+	    set.transform = transform;
+	    set.x = x;
+	    set.y = y;
+	    set.flags = flags;
+	    if (!xf86CrtcSet(crtc, &set))
 	    {
 		crtc->enabled = save_enabled;
 		for (o = 0; o < config->num_output; o++)
-- 
1.7.2.3



More information about the xorg-devel mailing list