RE: GTK progressbar call on XRenderComposite

[Huang, FrankR]

Ok. The test is huge for the format. The whole format number(nformats) is 16, 
that includes a8r8g8b8,x8r8g8b8,a8,r5g6b5...x2b10g10r10. I think your suggest's 
format is used often. So I will point these format to tests. And maybe use one 
color. And test blend,composite,cacompoiste with Src(Dst,Over,Clear)...
I'll go on:)


Thanks,
Frank

-Original Message-
From: Jonathan Morton [mailto:jonathan.mor...@movial.com] 
Sent: 2010年5月21日 14:59
To: Huang, FrankR
Cc: xorg-devel@lists.x.org; gtk-l...@gnome.org
Subject: RE: GTK progressbar call on XRenderComposite

On Fri, 2010-05-21 at 14:28 +0800, Huang, FrankR wrote:
 I decided to use rendercheck application to correct the rendering
 error in Geode drivers.  With this miscellaneous app, I will test
 every operation, every pict format. 
 Use -o and -t option to minimize what I want.
 Do you know the -f option use?

I would test a8r8g8b8 first, then add x8r8g8b8, a8 and r5g6b5 to the
list.  Probably once you fix the logic for those, the rest should follow
naturally.

-- 
--
From: Jonathan Morton
  jonathan.mor...@movial.com




___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

RE: Do not display the screensaver

[Cui, Hunk]

Hi, All,
Through communication with you, I have a certain understanding of this 
Gamma correction RAM (PAR  PDR registers) principle.
Now I use the ddd tools to debugging the xscreensaver-xserver, when I 
debug the server (about Get Gamma Ramp), The function: xf86GetGammaRamp - 
RRCrtcGammaGet - xf86RandR12CrtcGetGamma please see below:

The first line:
xf86CrtcPtr crtc = randr_crtc-devPrivate;

After run upper line,
crtc-gamma_red, crtc-gamma_green, crtc-gamma_blue tables have been loaded 
into the Gamma Correction values

Now I want to ask everyone, In what is the definition about the  
randr_crtc-devPrivate , where the values are arise in?

Thanks,
Hunk Cui

-Original Message-
From: yang...@gmail.com [mailto:yang...@gmail.com] On Behalf Of Yang Zhao
Sent: Thursday, May 20, 2010 12:39 PM
To: Cui, Hunk
Cc: xorg-devel@lists.x.org
Subject: Re: Do not display the screensaver

On 19 May 2010 21:03, Cui, Hunk hunk@amd.com wrote:
        What is mean about the server's internal representation is abstracted 
 away from the driver's representation? I don't understand it. Can you 
 particular explain it. I know the R,G,B originality values are 16 bits per 
 channel. When the values are transfered to the driver layer, they will be 
 dealed with through val = (*red  8) | *green | (*blue8); because the 
 val will be writen into Gamma correction RAM register (the type of hardware 
 register: Each of the entries are made up of corrections for R/G/B. Within 
 the DWORD, the red correction is in b[23:16], green in b[15:8] and blue in 
 b[7:0]).
        Why the driver is allowed to truncate? And why not be transfered by 
 R/G/B former values. Can you know that?

 BTW: Behrmann, Three one dimensional look up tables (LUT) each of 8-bit 
 resolution would not make much sense in this scenario. Please particular 
 explain it. Thank you for your earnest reply.

The term gamma in this discussion is actually misleading: all the
gamma-related calls, as they are currently implemented, eventually
results in writes to the hardware LUT which translates pixel values to
actual electrical output values. Gamma correction is just one of the
things you can do by modifying the values in the table.

The precision of the LUT depends on the hardware. Radeons, for
example, have 10 bits of precision per channel. CARD16 is an
appropriate upper bound for the range of precisions that will
realistically be in use.  Also keep in mind that these LUTs were used
primarily to drive analog outputs not too long ago, which have much,
much higher precisions than their digital counter parts.

A client makes a gamma correction call, server generates a new LUT
with 16 bits of precision per channel, then the driver takes this and
truncates to whatever precision the hardware can actually take.


-- 
Yang Zhao
http://yangman.ca

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

RE: GTK progressbar call on XRenderComposite

[Jonathan Morton]

On Fri, 2010-05-21 at 15:51 +0800, Huang, FrankR wrote:
 Ok. The test is huge for the format. The whole format number(nformats)
 is 16, that includes a8r8g8b8,x8r8g8b8,a8,r5g6b5...x2b10g10r10. I
 think your suggest's format is used often. So I will point these
 format to tests. And maybe use one color. And test
 blend,composite,cacompoiste with Src(Dst,Over,Clear)...
 I'll go on:)

I would leave out cacomposite and test repeat instead.

Indeed, test only one thing from each category at first, get those
right, and then expand.

-- 
--
From: Jonathan Morton
  jonathan.mor...@movial.com


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

RE: GTK progressbar call on XRenderComposite

[Huang, FrankR]

Ok. composite blend first.
Then expand to others.
Seem huge, but I think I can handle it :)
Hard begin, easy to follow.
But I don't know if the rendercheck can cover the most test cases.

Thanks,
Frank

-Original Message-
From: Jonathan Morton [mailto:jonathan.mor...@movial.com] 
Sent: 2010年5月21日 16:17
To: Huang, FrankR
Cc: xorg-devel@lists.x.org; gtk-l...@gnome.org
Subject: RE: GTK progressbar call on XRenderComposite

On Fri, 2010-05-21 at 15:51 +0800, Huang, FrankR wrote:
 Ok. The test is huge for the format. The whole format number(nformats)
 is 16, that includes a8r8g8b8,x8r8g8b8,a8,r5g6b5...x2b10g10r10. I
 think your suggest's format is used often. So I will point these
 format to tests. And maybe use one color. And test
 blend,composite,cacompoiste with Src(Dst,Over,Clear)...
 I'll go on:)

I would leave out cacomposite and test repeat instead.

Indeed, test only one thing from each category at first, get those
right, and then expand.

-- 
--
From: Jonathan Morton
  jonathan.mor...@movial.com




___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

RE: GTK progressbar call on XRenderComposite

[Huang, FrankR]

And after that, I think I can give patch to rendercheck:)

-Original Message-
From: Jonathan Morton [mailto:jonathan.mor...@movial.com] 
Sent: 2010年5月21日 16:17
To: Huang, FrankR
Cc: xorg-devel@lists.x.org; gtk-l...@gnome.org
Subject: RE: GTK progressbar call on XRenderComposite

On Fri, 2010-05-21 at 15:51 +0800, Huang, FrankR wrote:
 Ok. The test is huge for the format. The whole format number(nformats)
 is 16, that includes a8r8g8b8,x8r8g8b8,a8,r5g6b5...x2b10g10r10. I
 think your suggest's format is used often. So I will point these
 format to tests. And maybe use one color. And test
 blend,composite,cacompoiste with Src(Dst,Over,Clear)...
 I'll go on:)

I would leave out cacomposite and test repeat instead.

Indeed, test only one thing from each category at first, get those
right, and then expand.

-- 
--
From: Jonathan Morton
  jonathan.mor...@movial.com




___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] mi: removed the invisible cursor and never realize this cursor.

[Oliver McFadden]

Uggh, it appears this patch is ineffective compared to the previous
version. Turning on sprite debugging shows the Save/Restore functions
are still called.

[   162.260] SetCursor restore 2
[   162.264] SaveUnderCursor 2  
[   162.265] RestoreCursor 2
[   162.297] SetCursor restore 2
[   162.304] SaveUnderCursor 2  
[   162.304] RestoreCursor 2

I will check into it again.

On Fri, 2010-05-21 at 10:19 +0200, Mcfadden Oliver (Nokia-D/Helsinki)
wrote:
 On Fri, 2010-05-21 at 08:44 +0200, Mcfadden Oliver (Nokia-D/Helsinki)
 wrote:
  Previously we used an invisible cursor, however, this would still
  cause damage events and thus unnecessary redrawing. Now we never realize
  the cursor when it hasn't been defined (via XDefineCursor) or has been
  hidden (via XFixesHideCursor.)
  
  Improves performance when using software cursor sprites, primarily on
  devices where you do not want a visible cursor (touchscreen tablets,
  embedded devices, etc.)
  
  Signed-off-by: Oliver McFadden oliver.mcfad...@nokia.com
  ---
  v2 of [PATCH] mi: removed the invisible cursor sprite; use NullCursor 
  instead.
  
   xfixes/cursor.c |   46 --
   1 files changed, 4 insertions(+), 42 deletions(-)
  
  diff --git a/xfixes/cursor.c b/xfixes/cursor.c
  index 52bdb27..a52f3a0 100644
  --- a/xfixes/cursor.c
  +++ b/xfixes/cursor.c
  @@ -58,7 +58,6 @@ static RESTYPECursorClientType;
   static RESTYPE CursorHideCountType;
   static RESTYPE CursorWindowType;
   static CursorPtr   CursorCurrent[MAXDEVICES];
  -static CursorPtrpInvisibleCursor = NULL;
   
   static int CursorScreenPrivateKeyIndex;
   static DevPrivateKey CursorScreenPrivateKey = CursorScreenPrivateKeyIndex;
  @@ -135,7 +134,7 @@ CursorDisplayCursor (DeviceIntPtr pDev,
   CursorPtr pCursor)
   {
   CursorScreenPtrcs = GetCursorScreen(pScreen);
  -Bool   ret;
  +Bool   ret = TRUE;
   DisplayCursorProcPtr backupProc;
   
   Unwrap (cs, pScreen, DisplayCursor, backupProc);
  @@ -147,11 +146,9 @@ CursorDisplayCursor (DeviceIntPtr pDev,
   if (ConnectionInfo)
  CursorVisible = EnableCursor;
   
  -if (cs-pCursorHideCounts != NULL || !CursorVisible) {
  -ret = ((*pScreen-RealizeCursor)(pDev, pScreen, pInvisibleCursor) 
  
  -  (*pScreen-DisplayCursor) (pDev, pScreen, pInvisibleCursor));
  -} else {
  -   ret = (*pScreen-DisplayCursor) (pDev, pScreen, pCursor);
  +if (!cs-pCursorHideCounts  CursorVisible) {
  +ret = ((*pScreen-RealizeCursor)(pDev, pScreen, pCursor) 
  +  (*pScreen-DisplayCursor) (pDev, pScreen, pCursor));
   }
   
   if (pCursor != CursorCurrent[pDev-id])
  @@ -1031,37 +1028,6 @@ CursorFreeWindow (pointer data, XID id)
   return 1;
   }
   
  -static CursorPtr
  -createInvisibleCursor (void)
  -{
  -CursorPtr pCursor;
  -unsigned char *psrcbits, *pmaskbits;
  -CursorMetricRec cm;
  -
  -psrcbits = (unsigned char *) calloc(4, 1);
  -pmaskbits = (unsigned char *) calloc(4, 1);
  -if (psrcbits == NULL || pmaskbits == NULL) {
  -   return NULL;
  -}
  -
  -cm.width = 1;
  -cm.height = 1;
  -cm.xhot = 0;
  -cm.yhot = 0;
  -
  -if (AllocARGBCursor(psrcbits, pmaskbits,
  -   NULL, cm,
  -   0, 0, 0,
  -   0, 0, 0,
  -   pCursor, serverClient, (XID)0) != Success)
  -   return NullCursor;
  -
  -if (!AddResource(FakeClientID(0), RT_CURSOR, (pointer) pCursor))
  -   return NullCursor;
  -
  -return pCursor;
  -}
  -
   Bool
   XFixesCursorInit (void)
   {
  @@ -1090,10 +1056,6 @@ XFixesCursorInit (void)
   CursorWindowType = CreateNewResourceType(CursorFreeWindow,
   XFixesCursorWindow);
   
  -pInvisibleCursor = createInvisibleCursor();
  -if (pInvisibleCursor == NULL)
  -   return BadAlloc;
  -
   return CursorClientType  CursorHideCountType  CursorWindowType;
   }
   
 
 
 ___
 xorg-devel@lists.x.org: X.Org development
 Archives: http://lists.x.org/archives/xorg-devel
 Info: http://lists.x.org/mailman/listinfo/xorg-devel


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

RE: Do not display the screensaver

[Cui, Hunk]

Hi, All,
Through communication with you, I have a certain understanding of this 
Gamma correction RAM (PAR  PDR registers) principle.
Now I use the ddd tools to debugging the xscreensaver-xserver, when I 
debug the server (about Get Gamma Ramp), The function: xf86GetGammaRamp - 
RRCrtcGammaGet - xf86RandR12CrtcGetGamma please see below:

The first line:
xf86CrtcPtr crtc = randr_crtc-devPrivate;

After run upper line,
crtc-gamma_red, crtc-gamma_green, crtc-gamma_blue tables have been loaded 
into the Gamma Correction values

Now I want to ask everyone, In what is the definition about the  
randr_crtc-devPrivate , where the values are arise in?

[Cui, Hunk] The randr_crtc-devPrivate-gamma_red / 
randr_crtc-devPrivate-gamma_green / randr_crtc-devPrivate-gamma_blue are 
initialized in xf86InitialConfiguration - xf86CrtcSetInitialGamma function, 
is it?

Thanks,
Hunk Cui

-Original Message-
From: yang...@gmail.com [mailto:yang...@gmail.com] On Behalf Of Yang Zhao
Sent: Thursday, May 20, 2010 12:39 PM
To: Cui, Hunk
Cc: xorg-devel@lists.x.org
Subject: Re: Do not display the screensaver

On 19 May 2010 21:03, Cui, Hunk hunk@amd.com wrote:
        What is mean about the server's internal representation is abstracted 
 away from the driver's representation? I don't understand it. Can you 
 particular explain it. I know the R,G,B originality values are 16 bits per 
 channel. When the values are transfered to the driver layer, they will be 
 dealed with through val = (*red  8) | *green | (*blue8); because the 
 val will be writen into Gamma correction RAM register (the type of hardware 
 register: Each of the entries are made up of corrections for R/G/B. Within 
 the DWORD, the red correction is in b[23:16], green in b[15:8] and blue in 
 b[7:0]).
        Why the driver is allowed to truncate? And why not be transfered by 
 R/G/B former values. Can you know that?

 BTW: Behrmann, Three one dimensional look up tables (LUT) each of 8-bit 
 resolution would not make much sense in this scenario. Please particular 
 explain it. Thank you for your earnest reply.

The term gamma in this discussion is actually misleading: all the
gamma-related calls, as they are currently implemented, eventually
results in writes to the hardware LUT which translates pixel values to
actual electrical output values. Gamma correction is just one of the
things you can do by modifying the values in the table.

The precision of the LUT depends on the hardware. Radeons, for
example, have 10 bits of precision per channel. CARD16 is an
appropriate upper bound for the range of precisions that will
realistically be in use.  Also keep in mind that these LUTs were used
primarily to drive analog outputs not too long ago, which have much,
much higher precisions than their digital counter parts.

A client makes a gamma correction call, server generates a new LUT
with 16 bits of precision per channel, then the driver takes this and
truncates to whatever precision the hardware can actually take.


-- 
Yang Zhao
http://yangman.ca

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 02/11] xfree86: bus: remove unused headers

[Tiago Vignatti]

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
 hw/xfree86/common/xf86Bus.c |2 --
 hw/xfree86/os-support/bus/Pci.c |1 -
 hw/xfree86/os-support/bus/Pci.h |2 --
 hw/xfree86/vbe/vbe.c|1 -
 4 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/hw/xfree86/common/xf86Bus.c b/hw/xfree86/common/xf86Bus.c
index 8a73925..69fbdff 100644
--- a/hw/xfree86/common/xf86Bus.c
+++ b/hw/xfree86/common/xf86Bus.c
@@ -49,8 +49,6 @@
 #include xf86_OSproc.h
 #include xf86VGAarbiter.h
 
-#include Pci.h
-
 /* Entity data */
 EntityPtr *xf86Entities = NULL;/* Bus slots claimed by drivers */
 int xf86NumEntities = 0;
diff --git a/hw/xfree86/os-support/bus/Pci.c b/hw/xfree86/os-support/bus/Pci.c
index b7fa25f..a0a597d 100644
--- a/hw/xfree86/os-support/bus/Pci.c
+++ b/hw/xfree86/os-support/bus/Pci.c
@@ -126,7 +126,6 @@
 
 #include errno.h
 #include signal.h
-#include X11/Xarch.h
 #include compiler.h
 #include xf86.h
 #include xf86Priv.h
diff --git a/hw/xfree86/os-support/bus/Pci.h b/hw/xfree86/os-support/bus/Pci.h
index 70c831f..e001c30 100644
--- a/hw/xfree86/os-support/bus/Pci.h
+++ b/hw/xfree86/os-support/bus/Pci.h
@@ -107,8 +107,6 @@
 #ifndef _PCI_H
 #define _PCI_H 1
 
-#include X11/Xarch.h
-#include X11/Xfuncproto.h
 #include xf86Pci.h
 #include xf86PciInfo.h
 
diff --git a/hw/xfree86/vbe/vbe.c b/hw/xfree86/vbe/vbe.c
index 9f9b743..3840bfe 100644
--- a/hw/xfree86/vbe/vbe.c
+++ b/hw/xfree86/vbe/vbe.c
@@ -17,7 +17,6 @@
 
 #include xf86.h
 #include vbe.h
-#include X11/Xarch.h
 #include X11/extensions/dpmsconst.h
 
 #define VERSION(x) VBE_VERSION_MAJOR(x),VBE_VERSION_MINOR(x)
-- 
1.6.0.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 03/11] xfree86: bus: move IOADDRESS definition to a common header file

[Tiago Vignatti]

IOADDRESS type definition encompass more than just pci code, so move it to
common.

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
 hw/xfree86/common/xf86str.h |1 +
 hw/xfree86/os-support/bus/xf86Pci.h |3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
index de1f1b6..485c15a 100644
--- a/hw/xfree86/common/xf86str.h
+++ b/hw/xfree86/common/xf86str.h
@@ -58,6 +58,7 @@ typedef uint64_t memType;
 typedef uintptr_t memType;
 #endif
 
+typedef unsigned long IOADDRESS;/* Must be large enough for a pointer */
 
 /* Video mode flags */
 
diff --git a/hw/xfree86/os-support/bus/xf86Pci.h 
b/hw/xfree86/os-support/bus/xf86Pci.h
index ce1336b..a9a2779 100644
--- a/hw/xfree86/os-support/bus/xf86Pci.h
+++ b/hw/xfree86/os-support/bus/xf86Pci.h
@@ -235,7 +235,6 @@
 
 /* Primitive Types */
 typedef unsigned long ADDRESS; /* Memory/PCI address */
-typedef unsigned long IOADDRESS;   /* Must be large enough for a pointer */
 typedef unsigned long PCITAG;
 
 typedef enum {
@@ -257,6 +256,6 @@ extern _X_EXPORT Bool xf86scanpci(void);
 /* Domain access functions.  Some of these probably shouldn't be public */
 extern _X_EXPORT pointer xf86MapDomainMemory(int ScreenNum, int Flags, struct 
pci_device *dev,
 ADDRESS Base, unsigned long Size);
-extern _X_EXPORT IOADDRESS xf86MapLegacyIO(struct pci_device *dev);
+extern _X_EXPORT unsigned long xf86MapLegacyIO(struct pci_device *dev);
 
 #endif /* _XF86PCI_H */
-- 
1.6.0.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 04/11] xfree86: bus: remove superfluous and confused structures in BusRec

[Tiago Vignatti]

Although API is break, luckily any drivers right now is using such monster.

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
 hw/xfree86/common/xf86Bus.c|   16 
 hw/xfree86/common/xf86Helper.c |2 +-
 hw/xfree86/common/xf86VGAarbiter.c |2 +-
 hw/xfree86/common/xf86pciBus.c |   16 
 hw/xfree86/common/xf86sbusBus.c|8 
 hw/xfree86/common/xf86str.h|   10 ++
 6 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/hw/xfree86/common/xf86Bus.c b/hw/xfree86/common/xf86Bus.c
index 69fbdff..943574a 100644
--- a/hw/xfree86/common/xf86Bus.c
+++ b/hw/xfree86/common/xf86Bus.c
@@ -54,7 +54,7 @@ EntityPtr *xf86Entities = NULL;   /* Bus slots claimed by 
drivers */
 int xf86NumEntities = 0;
 static int xf86EntityPrivateCount = 0;
 
-BusRec primaryBus = { BUS_NONE, { 0 } };
+BusRec primaryBus = {BUS_NONE, 0, 0};
 
 /**
  * Call the driver's correct probe function.
@@ -261,9 +261,9 @@ xf86IsEntityPrimary(int entityIndex)
 
 switch (pEnt-bus.type) {
 case BUS_PCI:
-   return (pEnt-bus.id.pci == primaryBus.id.pci);
+   return (pEnt-bus.pci == primaryBus.pci);
 case BUS_SBUS:
-   return (pEnt-bus.id.sbus.fbNum == primaryBus.id.sbus.fbNum);
+   return (pEnt-bus.sbus == primaryBus.sbus);
 default:
return FALSE;
 }
@@ -572,14 +572,14 @@ xf86FindPrimaryDevice(void)
case BUS_PCI:
bus = PCI;
snprintf(loc, sizeof(loc),  %2...@%2.2x:%2.2x:%1.1x,
-primaryBus.id.pci-bus,
-primaryBus.id.pci-domain,
-primaryBus.id.pci-dev,
-primaryBus.id.pci-func);
+primaryBus.pci-bus,
+primaryBus.pci-domain,
+primaryBus.pci-dev,
+primaryBus.pci-func);
break;
case BUS_SBUS:
bus = SBUS;
-   snprintf(loc, sizeof(loc),  %2.2x, primaryBus.id.sbus.fbNum);
+   snprintf(loc, sizeof(loc),  %2.2x, primaryBus.sbus);
break;
default:
bus = ;
diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
index 9ec5941..8fa2a01 100644
--- a/hw/xfree86/common/xf86Helper.c
+++ b/hw/xfree86/common/xf86Helper.c
@@ -1904,7 +1904,7 @@ xf86MatchPciInstances(const char *driverName, int 
vendorID,
EntityPtr pEnt = xf86Entities[j];
if (pEnt-bus.type != BUS_PCI)
continue;
-   if (pEnt-bus.id.pci == pPci) {
+   if (pEnt-bus.pci == pPci) {
retEntities[numFound - 1] = j;
xf86AddDevToEntity(j, instances[i].dev);
break;
diff --git a/hw/xfree86/common/xf86VGAarbiter.c 
b/hw/xfree86/common/xf86VGAarbiter.c
index 4a736fc..7d1fcc8 100644
--- a/hw/xfree86/common/xf86VGAarbiter.c
+++ b/hw/xfree86/common/xf86VGAarbiter.c
@@ -133,7 +133,7 @@ xf86VGAarbiterScrnInit(ScrnInfoPtr pScrn)
 if (pEnt-bus.type != BUS_PCI)
return;
 
-dev = pEnt-bus.id.pci;
+dev = pEnt-bus.pci;
 pScrn-vgaDev = dev;
 }
 
diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c
index 4656f1a..5e727ad 100644
--- a/hw/xfree86/common/xf86pciBus.c
+++ b/hw/xfree86/common/xf86pciBus.c
@@ -118,7 +118,7 @@ xf86PciProbe(void)
 #ifdef HAVE_PCI_DEVICE_IS_BOOT_VGA
if (pci_device_is_boot_vga(info)) {
 primaryBus.type = BUS_PCI;
-primaryBus.id.pci = info;
+primaryBus.pci = info;
 }
 #endif
info-user_data = 0;
@@ -138,7 +138,7 @@ xf86PciProbe(void)
 ((num == 1) || IS_VGA(info-device_class))) {
if (primaryBus.type == BUS_NONE) {
primaryBus.type = BUS_PCI;
-   primaryBus.id.pci = info;
+   primaryBus.pci = info;
} else {
xf86Msg(X_NOTICE,
More than one possible primary device found\n);
@@ -233,7 +233,7 @@ xf86ClaimPciSlot(struct pci_device * d, DriverPtr drvp,
p-driver = drvp;
p-chipset = chipset;
p-bus.type = BUS_PCI;
-   p-bus.id.pci = d;
+   p-bus.pci = d;
p-active = active;
p-inUse = FALSE;
if (dev)
@@ -261,7 +261,7 @@ xf86UnclaimPciSlot(struct pci_device *d)
 for (i = 0; i  xf86NumEntities; i++) {
const EntityPtr p = xf86Entities[i];
 
-   if ((p-bus.type == BUS_PCI)  (p-bus.id.pci == d)) {
+   if ((p-bus.type == BUS_PCI)  (p-bus.pci == d)) {
/* Probably the slot should be deallocated? */
p-bus.type = BUS_NONE;
return;
@@ -368,7 +368,7 @@ xf86ComparePciBusString(const char *busID, int bus, int 
device, int func)
 Bool
 xf86IsPrimaryPci(struct pci_device *pPci)
 {
-return ((primaryBus.type == BUS_PCI)  (pPci == primaryBus.id.pci));
+return ((primaryBus.type == BUS_PCI)  (pPci == 

[PATCH 06/11] xfree86: bus: remove useless field from EntityRec

[Tiago Vignatti]

RAC is the champion of remaining trash for sure!

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
 hw/xfree86/common/xf86Bus.h |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/xfree86/common/xf86Bus.h b/hw/xfree86/common/xf86Bus.h
index b22e2e7..e161c7f 100644
--- a/hw/xfree86/common/xf86Bus.h
+++ b/hw/xfree86/common/xf86Bus.h
@@ -54,7 +54,6 @@ typedef struct {
 Boolactive;
 BoolinUse;
 BusRec  bus;
-pointer busAcc;
 int lastScrnFlag;
 DevUnion *  entityPrivates;
 int numInstances;
-- 
1.6.0.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 07/11] xfree86: remove all kind of bus and PCI dependency from the common helper file

[Tiago Vignatti]

Move all PCI procedures from xf86Helper.c to a more meaningful place (namely
xf86pciBus.c). xf86Helper.c is free of PCI code now.

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
 hw/xfree86/common/xf86Helper.c |  503 
 hw/xfree86/common/xf86pciBus.c |  494 +++
 2 files changed, 494 insertions(+), 503 deletions(-)

diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
index 8fa2a01..134ca7e 100644
--- a/hw/xfree86/common/xf86Helper.c
+++ b/hw/xfree86/common/xf86Helper.c
@@ -38,9 +38,6 @@
 #include xorg-config.h
 #endif
 
-#include pciaccess.h
-#include Pci.h
-
 #include X11/X.h
 #include os.h
 #include servermd.h
@@ -57,7 +54,6 @@
 #include xf86Xinput.h
 #include xf86InPriv.h
 #include mivalidate.h
-#include xf86Bus.h
 #include xf86Crtc.h
 
 /* For xf86GetClocks */
@@ -1506,420 +1502,6 @@ xf86MatchDevice(const char *drivername, GDevPtr 
**sectlist)
 return i;
 }
 
-static Bool
-pciDeviceHasBars(struct pci_device *pci)
-{
-int i;
-
-for (i = 0; i  6; i++)
-   if (pci-regions[i].size)
-   return TRUE;
-
-if (pci-rom_size)
-   return TRUE;
-
-return FALSE;
-}
-
-struct Inst {
-struct pci_device *pci;
-GDevPtrdev;
-Bool   foundHW;  /* PCIid in list of supported chipsets */
-Bool   claimed;  /* BusID matches with a device section */
-intchip;
-intscreen;
-};
-
-
-/**
- * Find set of unclaimed devices matching a given vendor ID.
- *
- * Used by drivers to find as yet unclaimed devices matching the specified
- * vendor ID.
- *
- * \param driverName Name of the driver.  This is used to find Device
- *   sections in the config file.
- * \param vendorID   PCI vendor ID of associated devices.  If zero, then
- *   the true vendor ID must be encoded in the \c PCIid
- *   fields of the \c PCIchipsets entries.
- * \param chipsets   Symbol table used to associate chipset names with
- *   PCI IDs.
- * \param devListList of Device sections parsed from the config file.
- * \param numDevsNumber of entries in \c devList.
- * \param drvp   Pointer the driver's control structure.
- * \param foundEntities  Returned list of entity indicies associated with the
- *   driver.
- *
- * \returns
- * The number of elements in returned in \c foundEntities on success or zero
- * on failure.
- *
- * \todo
- * This function does a bit more than short description says.  Fill in some
- * more of the details of its operation.
- *
- * \todo
- * The \c driverName parameter is redundant.  It is the same as
- * \c DriverRec::driverName.  In a future version of this function, remove
- * that parameter.
- */
-int
-xf86MatchPciInstances(const char *driverName, int vendorID,
- SymTabPtr chipsets, PciChipsets *PCIchipsets,
- GDevPtr *devList, int numDevs, DriverPtr drvp,
- int **foundEntities)
-{
-int i,j;
-struct pci_device * pPci;
-struct pci_device_iterator *iter;
-struct Inst *instances = NULL;
-int numClaimedInstances = 0;
-int allocatedInstances = 0;
-int numFound = 0;
-SymTabRec *c;
-PciChipsets *id;
-int *retEntities = NULL;
-
-*foundEntities = NULL;
-
-
-/* Each PCI device will contribute at least one entry.  Each device
- * section can contribute at most one entry.  The sum of the two is
- * guaranteed to be larger than the maximum possible number of entries.
- * Do this calculation and memory allocation once now to eliminate the
- * need for realloc calls inside the loop.
- */
-if (!(xf86DoConfigure  xf86DoConfigurePass1)) {
-   unsigned max_entries = numDevs;
-
-   iter = pci_slot_match_iterator_create(NULL);
-   while ((pPci = pci_device_next(iter)) != NULL) {
-   max_entries++;
-   }
-
-   pci_iterator_destroy(iter);
-   instances = xnfalloc(max_entries * sizeof(struct Inst));
-}
-
-iter = pci_slot_match_iterator_create(NULL);
-while ((pPci = pci_device_next(iter)) != NULL) {
-   unsigned device_class = pPci-device_class;
-   Bool foundVendor = FALSE;
-
-
-   /* Convert the pre-PCI 2.0 device class for a VGA adapter to the
-* 2.0 version of the same class.
-*/
-   if ( device_class == 0x0101 ) {
-   device_class = 0x0003;
-   }
-
-
-   /* Find PCI devices that match the given vendor ID.  The vendor ID is
-* either specified explicitly as a parameter to the function or
-* implicitly encoded in the high bits of id-PCIid.
-*
-* The first device with a matching vendor is recorded, even if the
-* device ID doesn't match.  This is done because the Device section
-* in the xorg.conf file can 

[PATCH 08/11] xfree86: remove BUS_ISA type given we don't support anymore

[Tiago Vignatti]

Should go together within commit df14682a.

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
 hw/xfree86/common/xf86str.h |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
index e5713db..9e6b150 100644
--- a/hw/xfree86/common/xf86str.h
+++ b/hw/xfree86/common/xf86str.h
@@ -339,7 +339,6 @@ typedef struct _DriverRec {
 /* Tolerate prior #include linux/input.h */
 #if defined(linux)  defined(_INPUT_H)
 #undef BUS_NONE
-#undef BUS_ISA
 #undef BUS_PCI
 #undef BUS_SBUS
 #undef BUS_last
@@ -347,7 +346,6 @@ typedef struct _DriverRec {
 
 typedef enum {
 BUS_NONE,
-BUS_ISA,
 BUS_PCI,
 BUS_SBUS,
 BUS_last/* Keep last */
-- 
1.6.0.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 01/11] xfree86: bus: remove unused pci macros

[Tiago Vignatti]

Should be gone in commits 3c03d9f1 and a9d7d659a respectively.

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
 hw/xfree86/os-support/bus/Pci.h |6 --
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/hw/xfree86/os-support/bus/Pci.h b/hw/xfree86/os-support/bus/Pci.h
index b52a6cf..70c831f 100644
--- a/hw/xfree86/os-support/bus/Pci.h
+++ b/hw/xfree86/os-support/bus/Pci.h
@@ -121,9 +121,6 @@
 #define PCI_DOM_MASK 0x0ffu
 #endif
 
-#define DEVID(vendor, device) \
-((CARD32)((PCI_##device  16) | PCI_##vendor))
-
 #ifndef PCI_DOM_MASK
 # define PCI_DOM_MASK 0x0ffu
 #endif
@@ -143,9 +140,6 @@
 #define PCI_DEV_FROM_TAG(tag)  (((tag)  0xf800u)  11)
 #define PCI_FUNC_FROM_TAG(tag) (((tag)  0x0700u)  8)
 
-#define PCI_DFN_FROM_TAG(tag)  (((tag)  0xff00u)  8)
-#define PCI_BDEV_FROM_TAG(tag) ((tag)  0x00fff800u)
-
 #define PCI_DOM_FROM_BUS(bus)  (((bus)  8)  (PCI_DOM_MASK))
 #define PCI_BUS_NO_DOMAIN(bus) ((bus)  0xffu)
 #define PCI_TAG_NO_DOMAIN(tag) ((tag)  0x0000u)
-- 
1.6.0.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 10/11] xfree86: no need to check for the configuration case when matching devices

[Tiago Vignatti]

xf86MatchDevice will never be called in configuration time.

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
 hw/xfree86/common/xf86Helper.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
index 134ca7e..bde80ea 100644
--- a/hw/xfree86/common/xf86Helper.c
+++ b/hw/xfree86/common/xf86Helper.c
@@ -1440,8 +1440,6 @@ xf86MatchDevice(const char *drivername, GDevPtr 
**sectlist)
 if (sectlist)
*sectlist = NULL;
 
-if (xf86DoConfigure  xf86DoConfigurePass1) return 1;
-
 /*
  * This is a very important function that matches the device sections
  * as they show up in the config file with the drivers that the server
-- 
1.6.0.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 05/11] xfree86: bus: delete useless xf86FindPrimaryDevice

[Tiago Vignatti]

This function had a wrong name and was just logging the primary device. No one
cares about it honestly.

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
 hw/xfree86/common/xf86Bus.c   |   37 -
 hw/xfree86/common/xf86Configure.c |2 --
 hw/xfree86/common/xf86Priv.h  |4 
 3 files changed, 0 insertions(+), 43 deletions(-)

diff --git a/hw/xfree86/common/xf86Bus.c b/hw/xfree86/common/xf86Bus.c
index 943574a..7aa2540 100644
--- a/hw/xfree86/common/xf86Bus.c
+++ b/hw/xfree86/common/xf86Bus.c
@@ -111,10 +111,6 @@ xf86BusConfig(void)
 if (xorgHWAccess)
 xorgHWAccess = xf86EnableIO();
 
-/* Locate bus slot that had register IO enabled at server startup */
-if (xorgHWAccess)
-xf86FindPrimaryDevice();
-
 /*
  * Now call each of the Probe functions.  Each successful probe will
  * result in an extra entry added to the xf86Screens[] list for each
@@ -557,39 +553,6 @@ xf86PostScreenInit(void)
 xf86EnterServerState(OPERATING);
 }
 
-/*
- * xf86FindPrimaryDevice() - Find the display device which
- * was active when the server was started.
- */
-void
-xf86FindPrimaryDevice(void)
-{
-if (primaryBus.type != BUS_NONE) {
-   char *bus;
-   char loc[16];
-
-   switch (primaryBus.type) {
-   case BUS_PCI:
-   bus = PCI;
-   snprintf(loc, sizeof(loc),  %2...@%2.2x:%2.2x:%1.1x,
-primaryBus.pci-bus,
-primaryBus.pci-domain,
-primaryBus.pci-dev,
-primaryBus.pci-func);
-   break;
-   case BUS_SBUS:
-   bus = SBUS;
-   snprintf(loc, sizeof(loc),  %2.2x, primaryBus.sbus);
-   break;
-   default:
-   bus = ;
-   loc[0] = '\0';
-   }
-
-   xf86MsgVerb(X_INFO, 2, Primary Device is: %s%s\n,bus,loc);
-}
-}
-
 int
 xf86GetLastScrnFlag(int entityIndex)
 {
diff --git a/hw/xfree86/common/xf86Configure.c 
b/hw/xfree86/common/xf86Configure.c
index 3013321..2f93bb1 100644
--- a/hw/xfree86/common/xf86Configure.c
+++ b/hw/xfree86/common/xf86Configure.c
@@ -681,8 +681,6 @@ DoConfigure(void)
xorgHWAccess = FALSE;
 }
 
-xf86FindPrimaryDevice();
- 
 /* Create XF86Config file structure */
 xf86config = calloc(1, sizeof(XF86ConfigRec));
 
diff --git a/hw/xfree86/common/xf86Priv.h b/hw/xfree86/common/xf86Priv.h
index d2073ae..b5e7a45 100644
--- a/hw/xfree86/common/xf86Priv.h
+++ b/hw/xfree86/common/xf86Priv.h
@@ -110,14 +110,10 @@ extern _X_EXPORT  RootWinPropPtr 
*xf86RegisteredPropertiesTable;
 #ifndef _NO_XF86_PROTOTYPES
 
 /* xf86Bus.c */
-
 extern _X_EXPORT Bool xf86BusConfig(void);
 extern _X_EXPORT void xf86BusProbe(void);
 extern _X_EXPORT void xf86AccessEnter(void);
 extern _X_EXPORT void xf86AccessLeave(void);
-
-extern _X_EXPORT void xf86FindPrimaryDevice(void);
-/* new RAC */
 extern _X_EXPORT void xf86PostProbe(void);
 extern _X_EXPORT void xf86ClearEntityListForScreen(int scrnIndex);
 extern _X_EXPORT void xf86AddDevToEntity(int entityIndex, GDevPtr dev);
-- 
1.6.0.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 09/11] xfree86: organize and group all pci related stuff inside xf86.h

[Tiago Vignatti]

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---
 hw/xfree86/common/xf86.h |   57 +
 1 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h
index 68c3744..d8629a8 100644
--- a/hw/xfree86/common/xf86.h
+++ b/hw/xfree86/common/xf86.h
@@ -41,8 +41,6 @@
 #include dix-config.h
 #endif
 
-#include pciaccess.h
-
 #include xf86str.h
 #include xf86Opt.h
 #include X11/Xfuncproto.h
@@ -62,7 +60,6 @@ extern _X_EXPORT DevPrivateKey xf86CreateRootWindowKey;
 extern _X_EXPORT DevPrivateKey xf86PixmapKey;
 extern _X_EXPORT ScrnInfoPtr *xf86Screens; /* List of pointers to 
ScrnInfoRecs */
 extern _X_EXPORT const unsigned char byte_reversed[256];
-extern _X_EXPORT Bool pciSlotClaimed;
 extern _X_EXPORT Bool fbSlotClaimed;
 #if (defined(__sparc__) || defined(__sparc))  !defined(__OpenBSD__)
 extern _X_EXPORT Bool sbusSlotClaimed;
@@ -91,21 +88,39 @@ extern _X_EXPORT Bool VTSwitchEnabled;  /* kbd driver */
 /* Function Prototypes */
 #ifndef _NO_XF86_PROTOTYPES
 
-/* xf86Bus.c */
+/* PCI related */
+#include pciaccess.h
+extern _X_EXPORT Bool pciSlotClaimed;
 
-extern _X_EXPORT Bool xf86CheckPciSlot( const struct pci_device * );
-extern _X_EXPORT int xf86ClaimPciSlot( struct pci_device *, DriverPtr drvp,
-int chipset, GDevPtr dev, Bool active);
+extern _X_EXPORT Bool xf86CheckPciSlot(const struct pci_device *);
+extern _X_EXPORT int xf86ClaimPciSlot(struct pci_device *, DriverPtr drvp,
+  int chipset, GDevPtr dev, Bool active);
 extern _X_EXPORT void xf86UnclaimPciSlot(struct pci_device *);
-extern _X_EXPORT Bool xf86ParsePciBusString(const char *busID, int *bus, int 
*device,
-  int *func);
-extern _X_EXPORT Bool xf86ComparePciBusString(const char *busID, int bus, int 
device, int func);
+extern _X_EXPORT Bool xf86ParsePciBusString(const char *busID, int *bus,
+int *device, int *func);
+extern _X_EXPORT Bool xf86ComparePciBusString(const char *busID, int bus,
+  int device, int func);
 extern _X_EXPORT void xf86FormatPciBusNumber(int busnum, char *buffer);
+extern _X_EXPORT Bool xf86IsPrimaryPci(struct pci_device * pPci);
+extern _X_EXPORT Bool xf86CheckPciMemBase(struct pci_device * pPci,
+  memType base);
+extern _X_EXPORT struct pci_device * xf86GetPciInfoForEntity(int entityIndex);
+extern _X_EXPORT int xf86MatchPciInstances(const char *driverName,
+int vendorID, SymTabPtr chipsets, PciChipsets *PCIchipsets,
+GDevPtr *devList, int numDevs, DriverPtr drvp, int **foundEntities);
+extern _X_EXPORT ScrnInfoPtr xf86ConfigPciEntity(ScrnInfoPtr pScrn,
+int scrnFlag, int entityIndex,PciChipsets *p_chip, void *dummy,
+EntityProc init, EntityProc enter, EntityProc leave, pointer private);
+/* Obsolete! don't use */
+extern _X_EXPORT Bool xf86ConfigActivePciEntity(ScrnInfoPtr pScrn,
+int entityIndex,PciChipsets *p_chip, void *dummy, EntityProc init,
+EntityProc enter, EntityProc leave, pointer private);
+
+/* xf86Bus.c */
+
 extern _X_EXPORT int  xf86GetFbInfoForScreen(int scrnIndex);
 extern _X_EXPORT int xf86ClaimFbSlot(DriverPtr drvp, int chipset, GDevPtr dev, 
Bool active);
 extern _X_EXPORT int xf86ClaimNoSlot(DriverPtr drvp, int chipset, GDevPtr dev, 
Bool active);
-extern _X_EXPORT Bool xf86IsPrimaryPci(struct pci_device * pPci);
-/* new RAC */
 extern _X_EXPORT Bool xf86DriverHasEntities(DriverPtr drvp);
 extern _X_EXPORT void xf86AddEntityToScreen(ScrnInfoPtr pScrn, int 
entityIndex);
 extern _X_EXPORT void xf86SetEntityInstanceForScreen(ScrnInfoPtr pScrn, int 
entityIndex,
@@ -114,10 +129,8 @@ extern _X_EXPORT int xf86GetNumEntityInstances(int 
entityIndex);
 extern _X_EXPORT GDevPtr xf86GetDevFromEntity(int entityIndex, int instance);
 extern _X_EXPORT void xf86RemoveEntityFromScreen(ScrnInfoPtr pScrn, int 
entityIndex);
 extern _X_EXPORT EntityInfoPtr xf86GetEntityInfo(int entityIndex);
-extern _X_EXPORT struct pci_device * xf86GetPciInfoForEntity(int entityIndex);
 extern _X_EXPORT Bool xf86SetEntityFuncs(int entityIndex, EntityProc init,
EntityProc enter, EntityProc leave, pointer);
-extern _X_EXPORT Bool xf86CheckPciMemBase(struct pci_device * pPci, memType 
base);
 extern _X_EXPORT Bool xf86IsEntityPrimary(int entityIndex);
 extern _X_EXPORT void xf86EnterServerState(xf86State state);
 extern _X_EXPORT ScrnInfoPtr xf86FindScreenForEntity(int entityIndex);
@@ -223,10 +236,6 @@ extern _X_EXPORT void xf86ShowClocks(ScrnInfoPtr scrp, 
MessageType from);
 extern _X_EXPORT void xf86PrintChipsets(const char *drvname, const char 
*drvmsg,
   SymTabPtr chips);
 extern _X_EXPORT int xf86MatchDevice(const char *drivername, GDevPtr 
**driversectlist);
-extern _X_EXPORT int xf86MatchPciInstances(const char 

Re: Scratch GC performance (was Re: [PATCH] dix: Reshuffle ScreenRec to pack holes)

[Adam Jackson]

On Thu, 2010-05-20 at 11:27 -0700, Jamey Sharp wrote:
 On Thu, May 20, 2010 at 11:17 AM, Adam Jackson a...@nwnk.net wrote:
  The numbers, sadly, are pretty clear:
 
  717000.0   518000.0 (  0.72)   Map window via parent (4 kids)
 
  That's not really a performance hit I'm willing to take.
 
 A 7%-28% hit? Yeah, not so much. Thanks for checking!
 
 Do you have a shortcut to working out which x11perf tests are going to
 hit a particular code path? I'd have been happy to test this myself if
 I'd had any idea which tests would be reasonable.

I pretty much just worked backwards by grepping for callers of
GetScratchGC.  I first looked at the general miPolyArc case and tried to
conjure up a test that would hit it.  x11perf -rop GXxor -wdcircle100
will do it, but the performance is (predictably) so abysmal that you
can't measure a difference; anything you can only do 2e3 of per second
isn't going to be affected by the overhead of one malloc per.

Window ops, on the other hand, are in the 2e6 per second range.  So when
I saw a match in miexpose.c, I connected the dots: exposure processing
will naturally need a scratch GC, because the server has to use one to
do the exposure painting, so any tests that generate exposure paints
will work.

 Running all of x11perf is too painful for a quick I wonder if...?

Irritatingly, part of this is because x11perf tries very diligently to
estimate test execution speed _before_ getting the actual numbers.  So
if you say -time 1, it will calibrate every test for ~5 seconds each
before running it for 1 second per rep.  Apparently, back in the dark
ages, we didn't have timer signals to handle that kind of crap for us.

- ajax


signature.asc
Description: This is a digitally signed message part
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 03/11] xfree86: bus: move IOADDRESS definition to a common header file

[Alan Coopersmith]

Tiago Vignatti wrote:
 IOADDRESS type definition encompass more than just pci code, so move it to
 common.
 
 Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
 ---
  hw/xfree86/common/xf86str.h |1 +
  hw/xfree86/os-support/bus/xf86Pci.h |3 +--
  2 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
 index de1f1b6..485c15a 100644
 --- a/hw/xfree86/common/xf86str.h
 +++ b/hw/xfree86/common/xf86str.h
 @@ -58,6 +58,7 @@ typedef uint64_t memType;
  typedef uintptr_t memType;
  #endif
  
 +typedef unsigned long IOADDRESS;/* Must be large enough for a pointer */

If it must be large enough for a pointer, why not use uintptr_t like the code
two lines before it?

-- 
-Alan Coopersmith-alan.coopersm...@oracle.com
 Oracle Solaris Platform Engineering: X Window System

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 04/11] xfree86: bus: remove superfluous and confused structures in BusRec

[Keith Packard]

On Fri, 21 May 2010 14:43:17 +0300, Tiago Vignatti tiago.vigna...@nokia.com 
wrote:

 Although API is break, luckily any drivers right now is using such monster.

 diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
 index 485c15a..e5713db 100644
 --- a/hw/xfree86/common/xf86str.h
 +++ b/hw/xfree86/common/xf86str.h
 @@ -355,16 +355,10 @@ typedef enum {
  
  struct pci_device;
  
 -typedef struct {
 -int  fbNum;
 -} SbusBusId;
 -
  typedef struct _bus {
  BusType type;
 -union {
 - struct pci_device *pci;
 - SbusBusId sbus;
 -} id;
 +struct pci_device *pci;
 +int sbus;
  } BusRec, *BusPtr;

This doeesn't make sense to me -- a bus is either an SBus or PCI, and so
sticking those alternatives in a union (and nicely tagged at that with
the 'type') is logical, even if it does add another level of naming to
accessing the elements.

-- 
keith.pack...@intel.com


pgplw8ia7v3TH.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 06/11] xfree86: bus: remove useless field from EntityRec

[Keith Packard]

On Fri, 21 May 2010 14:43:19 +0300, Tiago Vignatti tiago.vigna...@nokia.com 
wrote:
 RAC is the champion of remaining trash for sure!
 
 Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com

Reviewed-by: Keith Packard kei...@keithp.com

-- 
keith.pack...@intel.com


pgpnmhMUtcGTe.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH resent] configure: introduce --{enable, disable}-fontserver

[Keith Packard]

On Wed,  5 May 2010 17:14:41 +0300, Tiago Vignatti tiago.vigna...@nokia.com 
wrote:

 Keith, no one took this one. Care to pull it?

I though I had replied suggesting that this should depend on what support was
provided in the system libXfont instead of having the server be
configured separately.

-- 
keith.pack...@intel.com


pgpNXMpNGiXY9.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 03/11] xfree86: bus: move IOADDRESS definition to a common header file

[Mark Kettenis]

 From: Tiago Vignatti tiago.vigna...@nokia.com
 Date: Fri, 21 May 2010 14:43:16 +0300
 
 IOADDRESS type definition encompass more than just pci code, so move it to
 common.
 
 diff --git a/hw/xfree86/os-support/bus/xf86Pci.h 
 b/hw/xfree86/os-support/bus/xf86Pci.h
 index ce1336b..a9a2779 100644
 --- a/hw/xfree86/os-support/bus/xf86Pci.h
 +++ b/hw/xfree86/os-support/bus/xf86Pci.h
 @@ -235,7 +235,6 @@
  
  /* Primitive Types */
  typedef unsigned long ADDRESS;   /* Memory/PCI address */
 -typedef unsigned long IOADDRESS; /* Must be large enough for a pointer */
  typedef unsigned long PCITAG;

Moving IOADDRESS over, but leaving the other ibviously related ones
behind, seems a bad idea to me.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] dix: add 3x3 transformation matrix xinput property for multi-head handling

[Keith Packard]

On Wed,  5 May 2010 17:54:07 +0200, Peter Korsgaard peter.korsga...@barco.com 
wrote:

 This is equivalent to the evdev patch sent earlier:

I'd like to see someone from the input team review this before I include
it in the server. In particular, the last I heard, there was a
suggestion that we should use this matrix for relative devices as well,
and either pull out a 2x2 subset or just document that non-affine or
translating transforms probably wouldn't do what you wanted.

-- 
keith.pack...@intel.com


pgpHiT9PJi4jc.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Replace screen-rgf scratch GC flags with a bit in each GC.

[Adam Jackson]

On Thu, 2010-05-20 at 10:46 -0700, Jamey Sharp wrote:
 This eliminates a poorly-named, poorly-documented field from the
 ScreenRec, using a previously-unused flag bit in each GC instead.
 
 Signed-off-by: Jamey Sharp ja...@minilop.net
 Cc: Keith Packard kei...@keithp.com
 Reviewed-by: Keith Packard kei...@keithp.com

Purple is the new black.

Reviewed-by: Adam Jackson a...@redhat.com

- ajax


signature.asc
Description: This is a digitally signed message part
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] vbe.h: Use __attribute__((packed)) on Sun cc 5.9 later as well as gcc

[Adam Jackson]

On Thu, 2010-05-20 at 17:56 -0700, Alan Coopersmith wrote:
 Signed-off-by: Alan Coopersmith alan.coopersm...@oracle.com

Reviewed-by: Adam Jackson a...@redhat.com

- ajax


signature.asc
Description: This is a digitally signed message part
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Solaris: Use VT_SET_CONSUSER ioctl to set Console User rights profile

[Adam Jackson]

On Thu, 2010-05-20 at 17:56 -0700, Alan Coopersmith wrote:
 From: Aaron Zang aaron.z...@sun.com
 
 When Xorg is started on display :0, this ioctl is called to grant the
 user the rights traditionally associated with /dev/console (before VT
 support was added), such as access to local peripheral devices.
 
 Also adds a Solaris-specific -C flag to force starting on /dev/console
 instead of /dev/vt*, allowing programs like xterm -C to access the
 console device.
 
 Signed-off-by: Alan Coopersmith alan.coopersm...@oracle.com

Curious API.

Reviewed-by: Adam Jackson a...@redhat.com

- ajax


signature.asc
Description: This is a digitally signed message part
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/2] composite: Don't backfill non-bg-None windows

[Keith Packard]

On Thu, 06 May 2010 10:30:25 -0400, Adam Jackson a...@nwnk.net wrote:
 On Wed, 2010-05-05 at 15:45 -0700, Keith Packard wrote:
  On Wed,  5 May 2010 16:25:28 -0400, Adam Jackson a...@redhat.com wrote:
  
   +
   +/* if we don't need to backfill, we're done */
   +if (pWin-backgroundState != BackgroundPixmap)
   +return pPixmap;
   +if (pWin-background.pixmap != None)
   +return pPixmap;
   +
  
  Presumably this pixmap is being allocated for the window manager frame,
  not the application itself. This isn't looking at the background for the
  application, just the frame.
 
 Oh, gross.  If I'm reading this right, miPaintWindow clips by children
 (not IncludeInferiors, and called after map and validate), which means
 if you map an entire subtree at once, the bg=None regions poke through.
 Is that accurate?
 
 I'm not totally convinced that's required by the protocol, but I can
 imagine it being required by applications.  It's easy enough to check,
 and the tree walk will almost certainly be faster than the blit.

If background None is specified, the window has no defined background.

When no valid contents are available for regions of a window and the
 regions are either visible or the server is maintaining backing store,
 the server automatically tiles the regions with the window's background
 unless the window has a background of None.  If the background is None,
 the previous screen contents from other windows of the same depth as the
 window are simply left in place if the contents come from the parent of
 the window or an inferior of the parent; otherwise, the initial contents
 of the exposed regions are undefined.

The first offers plenty of wiggle room here, the second, alas, the
second appears to require the tree walking plan. And, yes, lots of
applications depend on the contents being copied from the screen. In
particular menus often get mapped using background None and no contents
so that the first view seen is of the painted menu and not an empty
background.

Let me know if you're planning on providing a new patch for this.

-- 
keith.pack...@intel.com


pgpcXRqNfCB6P.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 03/11] xfree86: bus: move IOADDRESS definition to a common header file

[Adam Jackson]

On Fri, 2010-05-21 at 07:38 -0700, Keith Packard wrote:
 On Fri, 21 May 2010 14:43:16 +0300, Tiago Vignatti tiago.vigna...@nokia.com 
 wrote:
 
  -extern _X_EXPORT IOADDRESS xf86MapLegacyIO(struct pci_device *dev);
  +extern _X_EXPORT unsigned long xf86MapLegacyIO(struct pci_device *dev);
 
 'unsigned long'? Surely there's a type which more accurately reflects
 what this function returns.

Oh god, you really don't want to know.  It's a total tragedy.

- ajax


signature.asc
Description: This is a digitally signed message part
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Yet another reason window pictures suck

[Adam Jackson]

Context:

https://bugzilla.redhat.com/show_bug.cgi?id=533879

If you've got a 24bpp framebuffer, you poor misguided soul you, then you
have a conundrum.  Pixmaps are 32bpp but Windows aren't.  Suppose you
make a Picture out of a Window.  You would think you'd be required to
use one of the Formats with an associated Visual.  What does the spec
say about it?

QueryPictFormats
The server responds with a list of supported PictFormats and
a list of which PictFormat goes with each visual on each screen.
Every PictFormat must match a supported depth, but not every
PictFormat need have a matching visual.

...

CreatePicture
This request creates a Picture object associated with the specified
drawable and assigns the identifier pid to it.  Pixel data in the
image are interpreted according to 'format'.  It is a Match error
to specify a format with a different depth than the drawable.  If
the drawable is a Window then the Red, Green and Blue masks must
match those in the visual for the window else a Match error is
generated.

So no, you don't actually have to pick a Visual Format for Window
Pictures.  You just have to pick a Format whose channels match.  Which
means you can pick an [AX]8R8G8B8 Format, as long as the color channels
are the right way around.  We'll then dutifully smash the drawable's bpp
into the top 8 bits of the format code, so now you have a 24bpp
[AX]RGB32 picture.  Pixman will use the channel widths to compute pixel
width, and that won't match your storage, and it all goes quite shitty
from there.

So what do we do here?  I see two options:

a) Do something like fb24_32 to munge the format code on the way in and
out of Render operations on 24/24 windows, and (presumably) treat them
as though alpha=1.

b) Fix the spec to be more strict, and fix cairo and whoever else to be
more careful about picking Visual Formats for Window Pictures.

Both are doable?  I lean towards the latter, since we've been carrying
this bug for ~10 years and it's only now really showing up.

- ajax


signature.asc
Description: This is a digitally signed message part
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] mi: removed the invisible cursor and never realize this cursor.

[Jamey Sharp]

On Thu, May 20, 2010 at 11:44 PM, Oliver McFadden
oliver.mcfad...@nokia.com wrote:
 -    if (cs-pCursorHideCounts != NULL || !CursorVisible) {
 -        ret = ((*pScreen-RealizeCursor)(pDev, pScreen, pInvisibleCursor) 
 -              (*pScreen-DisplayCursor) (pDev, pScreen, pInvisibleCursor));
 -    } else {
 -       ret = (*pScreen-DisplayCursor) (pDev, pScreen, pCursor);
 +    if (!cs-pCursorHideCounts  CursorVisible) {
 +        ret = ((*pScreen-RealizeCursor)(pDev, pScreen, pCursor) 
 +              (*pScreen-DisplayCursor) (pDev, pScreen, pCursor));
     }

I don't understand the intent here. I think you still want to call
DisplayCursor with NullCursor, just not RealizeCursor; and I guess
this patch introduces an extra call to RealizeCursor for the
visible-cursor case.

I thought you'd want this patch hunk instead:

diff --git a/xfixes/cursor.c b/xfixes/cursor.c
index 52bdb27..1f5f841 100644
--- a/xfixes/cursor.c
+++ b/xfixes/cursor.c
@@ -148,8 +148,7 @@ CursorDisplayCursor (DeviceIntPtr pDev,
CursorVisible = EnableCursor;

 if (cs-pCursorHideCounts != NULL || !CursorVisible) {
-ret = ((*pScreen-RealizeCursor)(pDev, pScreen, pInvisibleCursor) 
-  (*pScreen-DisplayCursor) (pDev, pScreen, pInvisibleCursor));
+ret = (*pScreen-DisplayCursor) (pDev, pScreen, NullCursor);
 } else {
ret = (*pScreen-DisplayCursor) (pDev, pScreen, pCursor);
 }

I haven't compiled that though, let alone tested it.

Jamey
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: State of the 1.9 release

[Vignatti Tiago (Nokia-D/Helsinki)]

Hi,

On Thu, May 20, 2010 at 05:02:15PM +0200, ext Keith Packard wrote:
 
 git://people.freedesktop.org/~keithp/xserver fix-private-usage

I see that you're registering some structures that are used only in xfree86
ddx using dix devPrivates. But for this ddx seems we have a similar mechanism
to of privates inside ScrnInfoRec (xf86AllocateScrnInfoPrivateIndex, or just
everything that uses DevUnion).

So makes sense to deprecate the ddx one and use only dix?

Even so:

Tested-by: Tiago Vignatti tiago.vigna...@nokia.com


Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] render: Avoid infinite loops in alpha map handling (#23581)

[Keith Packard]

On Tue, 11 May 2010 14:12:52 -0400, Adam Jackson a...@nwnk.net wrote:

 I just want it to not trivially crash my server.  I could care less
 about correctness for this.

How about this?

From 50be5e8ff00e734506d2bb0c25054d7648f0cf1e Mon Sep 17 00:00:00 2001
From: Keith Packard kei...@keithp.com
Date: Fri, 21 May 2010 09:01:43 -0700
Subject: [PATCH] Don't let alpha maps recurse in fb. Bug 23581.

Recursive alpha maps (where one picture's alpha map is set to a
picture with an external alpha map) would be all fine and dandy,
except for the case where the client constructs a loop. Detecting this
case when setting the alpha map values would be difficult as any time
an alpha map is set, the server would have to check for the looping
case.

Instead, a far simpler fix is to simply disallow recursive alpha maps
in the rendering code, the Render spec is ambiguous in this area and
allows us to to ignore the recursive case.

Signed-off-by: Keith Packard kei...@keithp.com
---
 fb/fbpict.c |   24 ++--
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/fb/fbpict.c b/fb/fbpict.c
index 896d33e..18a5204 100644
--- a/fb/fbpict.c
+++ b/fb/fbpict.c
@@ -332,8 +332,11 @@ create_bits_picture (PicturePtr pict,
 return image;
 }
 
+static pixman_image_t *
+image_from_pict_internal (PicturePtr pict, Bool has_clip, int *xoff, int 
*yoff, Bool is_alpha_map);
+
 static void
-set_image_properties (pixman_image_t *image, PicturePtr pict, Bool has_clip, 
int *xoff, int *yoff)
+set_image_properties (pixman_image_t *image, PicturePtr pict, Bool has_clip, 
int *xoff, int *yoff, Bool is_alpha_map)
 {
 pixman_repeat_t repeat;
 pixman_filter_t filter;
@@ -382,10 +385,13 @@ set_image_properties (pixman_image_t *image, PicturePtr 
pict, Bool has_clip, int
 
 pixman_image_set_repeat (image, repeat);
 
-if (pict-alphaMap)
+/* Fetch alpha map unless 'pict' is being used
+ * as the alpha map for this operation
+ */
+if (pict-alphaMap  !is_alpha_map)
 {
int alpha_xoff, alpha_yoff;
-   pixman_image_t *alpha_map = image_from_pict (pict-alphaMap, FALSE, 
alpha_xoff, alpha_yoff);
+   pixman_image_t *alpha_map = image_from_pict_internal (pict-alphaMap, 
FALSE, alpha_xoff, alpha_yoff, TRUE);

pixman_image_set_alpha_map (
image, alpha_map, pict-alphaOrigin.x, pict-alphaOrigin.y);
@@ -417,8 +423,8 @@ set_image_properties (pixman_image_t *image, PicturePtr 
pict, Bool has_clip, int
 pixman_image_set_source_clipping (image, TRUE);
 }
 
-pixman_image_t *
-image_from_pict (PicturePtr pict, Bool has_clip, int *xoff, int *yoff)
+static pixman_image_t *
+image_from_pict_internal (PicturePtr pict, Bool has_clip, int *xoff, int 
*yoff, Bool is_alpha_map)
 {
 pixman_image_t *image = NULL;
 
@@ -452,11 +458,17 @@ image_from_pict (PicturePtr pict, Bool has_clip, int 
*xoff, int *yoff)
 }
 
 if (image)
-   set_image_properties (image, pict, has_clip, xoff, yoff);
+   set_image_properties (image, pict, has_clip, xoff, yoff, is_alpha_map);
 
 return image;
 }
 
+pixman_image_t *
+image_from_pict (PicturePtr pict, Bool has_clip, int *xoff, int *yoff)
+{
+return image_from_pict_internal (pict, has_clip, xoff, yoff, FALSE);
+}
+
 void
 free_pixman_pict (PicturePtr pict, pixman_image_t *image)
 {
-- 
1.7.1


 
 - ajax
Non-text part: application/pgp-signature

-- 
keith.pack...@intel.com


pgpFzEDQncLVa.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 2/2] xfree86: vgaarb: UNFINISHED: hide arbiter device inside dix private structure

[Tiago Vignatti]

Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
---

This patch is not working so far because ScreenRec is not allocated yet when
dixSetPrivate is being called inside xf86VGAarbiterScrnInit. AddScreen is only
called afterwards.

Now I can think in two choices: either I hide vgaarb device inside DDX
privates (from ScrnInfoRec), or; I try to call AddScreen before, trying to
keep very close from xf86AllocateScreen. As I mentioned in the email I just
sent, seems we might want to deprecate ScrnInfoRec privates.


What are you guys think is more sensible?


 hw/xfree86/common/xf86VGAarbiter.c |   24 +---
 hw/xfree86/common/xf86VGAarbiterPriv.h |   10 ++
 hw/xfree86/common/xf86str.h|1 -
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/hw/xfree86/common/xf86VGAarbiter.c 
b/hw/xfree86/common/xf86VGAarbiter.c
index 185b9c2..f7f3357 100644
--- a/hw/xfree86/common/xf86VGAarbiter.c
+++ b/hw/xfree86/common/xf86VGAarbiter.c
@@ -66,6 +66,8 @@ static DevPrivateKeyRec VGAarbiterScreenKeyRec;
 #define VGAarbiterScreenKey (VGAarbiterScreenKeyRec)
 static DevPrivateKeyRec VGAarbiterGCKeyRec;
 #define VGAarbiterGCKey (VGAarbiterGCKeyRec)
+static DevPrivateKeyRec VGAarbiterDevKeyRec;
+#define VGAarbiterDevKey (VGAarbiterDevKeyRec)
 
 static int vga_no_arb = 0;
 void
@@ -88,11 +90,13 @@ xf86VGAarbiterFini(void)
 void
 xf86VGAarbiterLock(int scrnIdx)
 {
-ScrnInfoPtr pScrn = xf86Screens[scrnIdx];
+   ScreenPtr pScreen = screenInfo.screens[scrnIdx];
+   struct pci_device *dev = (struct pci_device *)
+   dixLookupPrivate(pScreen-devPrivates, VGAarbiterDevKey);
 
 if (vga_no_arb)
return;
-pci_device_vgaarb_set_target(pScrn-vgaDev);
+pci_device_vgaarb_set_target(dev);
 pci_device_vgaarb_lock();
 }
 
@@ -106,14 +110,15 @@ xf86VGAarbiterUnlock(void)
 
 Bool xf86VGAarbiterAllowDRI(ScreenPtr pScreen)
 {
+struct pci_device *dev = (struct pci_device *)
+dixLookupPrivate(pScreen-devPrivates, VGAarbiterDevKey);
 int vga_count;
 int rsrc_decodes;
-ScrnInfoPtr pScrn = xf86Screens[pScreen-myNum];
 
 if (vga_no_arb)
return TRUE;
 
-pci_device_vgaarb_get_info(pScrn-vgaDev, vga_count, rsrc_decodes);
+pci_device_vgaarb_get_info(dev, vga_count, rsrc_decodes);
 if (vga_count  1) {
 if (rsrc_decodes) {
 return FALSE;
@@ -126,7 +131,7 @@ void
 xf86VGAarbiterScrnInit(int scrnIdx)
 {
 ScrnInfoPtr pScrn = xf86Screens[scrnIdx];
-struct pci_device *dev;
+   ScreenPtr pScreen = screenInfo.screens[scrnIdx];
 EntityPtr pEnt;
 
 if (vga_no_arb)
@@ -136,8 +141,13 @@ xf86VGAarbiterScrnInit(int scrnIdx)
 if (pEnt-bus.type != BUS_PCI)
return;
 
-dev = pEnt-bus.id.pci;
-pScrn-vgaDev = dev;
+if (!dixRegisterPrivateKey(VGAarbiterDevKeyRec, PRIVATE_SCREEN, 0)) {
+   FatalError(%s: private request failed.\n, __FUNCTION__);
+   return;
+   }
+
+dixSetPrivate(pScreen-devPrivates, VGAarbiterDevKey,
+ pEnt-bus.id.pci);
 }
 
 void
diff --git a/hw/xfree86/common/xf86VGAarbiterPriv.h 
b/hw/xfree86/common/xf86VGAarbiterPriv.h
index 9b4a597..4fc6703 100644
--- a/hw/xfree86/common/xf86VGAarbiterPriv.h
+++ b/hw/xfree86/common/xf86VGAarbiterPriv.h
@@ -96,12 +96,14 @@
 #define GC_SCREEN register ScrnInfoPtr pScrn = \
 xf86Screens[pGC-pScreen-myNum]
 
-#define VGAGet(x)\
-pci_device_vgaarb_set_target(xf86Screens[pScreen-myNum]-vgaDev); \
+#define VGAGet(x) struct pci_device *dev = \
+(struct pci_device *) dixLookupPrivate(pScreen-devPrivates, 
VGAarbiterDevKey); \
+pci_device_vgaarb_set_target(dev); \
 pci_device_vgaarb_lock();
 
-#define VGAGet_GC(x)\
-pci_device_vgaarb_set_target(xf86Screens[pGC-pScreen-myNum]-vgaDev); \
+#define VGAGet_GC(x) struct pci_device *dev = \
+(struct pci_device *) dixLookupPrivate(pGC-pScreen-devPrivates, 
VGAarbiterDevKey); \
+pci_device_vgaarb_set_target(dev); \
 pci_device_vgaarb_lock();
 
 #define VGAPut(x)\
diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
index de1f1b6..ef36c02 100644
--- a/hw/xfree86/common/xf86str.h
+++ b/hw/xfree86/common/xf86str.h
@@ -783,7 +783,6 @@ typedef struct _ScrnInfoRec {
 intreservedInt[NUM_RESERVED_INTS];
 
 int *  entityInstanceList;
-struct pci_device   *vgaDev;
 
 pointerreservedPtr[NUM_RESERVED_POINTERS];
 
-- 
1.6.0.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: DRI2 fixes

[Jesse Barnes]

On Fri, 21 May 2010 14:05:41 +1000
Peter Hutterer peter.hutte...@who-t.net wrote:

 On Thu, May 20, 2010 at 09:19:03AM -0700, Jesse Barnes wrote:
  On Mon, 29 Mar 2010 10:04:28 -0700
  Jesse Barnes jbar...@virtuousgeek.org wrote:
  
   On Mon, 22 Mar 2010 15:03:30 -0700
   Jesse Barnes jbar...@virtuousgeek.org wrote:
   
This is a collection of fixes from my personal server tree targeting the
1.8 release.  They're mostly small fixes, but they fix a few important
(i.e. common) cases with the new protocol code.

Please review; I'll make any necessary changes, add the Reviewed-by
tags, and push to Keith.
   
   Ok, just updated my tree (master branch at
   git://people.freedesktop.org/~jbarnes/xserver) with the reviewed-by
   tags after some testing.
   
   Please pull it, hopefully into 1.8 since it fixes some ugly bugs (e.g.
   handling MSC counts in the past w/o hanging), or into 1.8.1 if you've
   already done with 1.8.
  
  Peter, can you pull these fixes into 1.8.x?  It seems they got lost
  after hitting master; they're really needed in 1.8.x too.
 
 pushed, thanks. please double-check that I have merged the right commits, a
 pull request would have been a bit easier given that I know little to
 nothing of the DRI2 parts.

Ok checking now; I figured the commits hadn't changed since Keith
pulled them into something very similar to 1.8.  Hope you didn't have
to deal with any conflicts...

Commits look good, thanks.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: Yet another reason window pictures suck

[Keith Packard]

On Fri, 21 May 2010 17:10:59 +0100, Chris Wilson ch...@chris-wilson.co.uk 
wrote:

 The implementation should be fixed though, let's return BadMatch and
 stop this silent corruption.

If existing apps are using these formats, it seems reasonable to expect
us to make them work. It seems like the only piece necessary here would
be to munge the format code passed from the server to pixman. That seems
pretty easy to me.

-- 
keith.pack...@intel.com


pgp3M4kxnVOyr.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: State of the 1.9 release

[Keith Packard]

On Fri, 21 May 2010 19:02:32 +0300, Vignatti Tiago (Nokia-D/Helsinki) 
tiago.vigna...@nokia.com wrote:

 So makes sense to deprecate the ddx one and use only dix?

Alas, the ScrnInfoRec has a different lifetime than the ScreenRec, so
I'm not sure we could deprecate the DDX one. Would take some study.

 Even so:
 
 Tested-by: Tiago Vignatti tiago.vigna...@nokia.com

Thanks much! Is this on a multi-GPU system?

-- 
keith.pack...@intel.com


pgpLQKr8afe6B.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/2] xfree86: vgaarb: simplify the arguments passed to lock/unlock

[Keith Packard]

On Fri, 21 May 2010 18:48:14 +0300, Tiago Vignatti tiago.vigna...@nokia.com 
wrote:

 Send only screen index instead the whole rec for lock and remove the argument
 of unlock.

nak -- the arbiter presumably needs to run during server initialization
before the screen is allocated, which means that any per-GPU data will
end up hanging from the scrninfo in any case.

-- 
keith.pack...@intel.com


pgpCX8PO8pXny.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 03/11] xfree86: bus: move IOADDRESS definition to a common header file

[Keith Packard]

 Moving IOADDRESS over, but leaving the other ibviously related ones
 behind, seems a bad idea to me.

I'm waiting for an answer to the questions posed here before merging
this patch.

-- 
keith.pack...@intel.com


pgpcc5muSVNCr.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 04/11] xfree86: bus: remove superfluous and confused structures in BusRec

[Vignatti Tiago (Nokia-D/Helsinki)]

On Fri, May 21, 2010 at 04:40:59PM +0200, ext Keith Packard wrote:
 On Fri, 21 May 2010 14:43:17 +0300, Tiago Vignatti tiago.vigna...@nokia.com 
 wrote:
 
  Although API is break, luckily any drivers right now is using such monster.
 
  diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
  index 485c15a..e5713db 100644
  --- a/hw/xfree86/common/xf86str.h
  +++ b/hw/xfree86/common/xf86str.h
  @@ -355,16 +355,10 @@ typedef enum {
   
   struct pci_device;
   
  -typedef struct {
  -intfbNum;
  -} SbusBusId;
  -
   typedef struct _bus {
   BusType type;
  -union {
  -   struct pci_device *pci;
  -   SbusBusId sbus;
  -} id;
  +struct pci_device *pci;
  +int sbus;
   } BusRec, *BusPtr;
 
 This doeesn't make sense to me -- a bus is either an SBus or PCI, and so
 sticking those alternatives in a union (and nicely tagged at that with
 the 'type') is logical, even if it does add another level of naming to
 accessing the elements.

I'd say it does make sense but depends on the style of the programmer. But
c'mon, look again at this naming level:

-   return (pEnt-bus.id.sbus.fbNum == primaryBus.id.sbus.fbNum);
+   return (pEnt-bus.sbus == primaryBus.sbus);


the previous code seems pretty messy! 

Did I change your opinion? :)

Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 03/11] xfree86: bus: move IOADDRESS definition to a common header file

[Vignatti Tiago (Nokia-D/Helsinki)]

On Fri, May 21, 2010 at 04:21:26PM +0200, ext Alan Coopersmith wrote:
 Tiago Vignatti wrote:
  IOADDRESS type definition encompass more than just pci code, so move it to
  common.
  
  Signed-off-by: Tiago Vignatti tiago.vigna...@nokia.com
  ---
   hw/xfree86/common/xf86str.h |1 +
   hw/xfree86/os-support/bus/xf86Pci.h |3 +--
   2 files changed, 2 insertions(+), 2 deletions(-)
  
  diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
  index de1f1b6..485c15a 100644
  --- a/hw/xfree86/common/xf86str.h
  +++ b/hw/xfree86/common/xf86str.h
  @@ -58,6 +58,7 @@ typedef uint64_t memType;
   typedef uintptr_t memType;
   #endif
   
  +typedef unsigned long IOADDRESS;/* Must be large enough for a pointer 
  */
 
 If it must be large enough for a pointer, why not use uintptr_t like the code
 two lines before it?

I don't know. Seems uintptr_t is unsigned int in Linux, does it seems enough
to carry IO address there? Ajax?

Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 03/11] xfree86: bus: move IOADDRESS definition to a common header file

[Vignatti Tiago (Nokia-D/Helsinki)]

On Fri, May 21, 2010 at 04:55:23PM +0200, ext Mark Kettenis wrote:
  From: Tiago Vignatti tiago.vigna...@nokia.com
  Date: Fri, 21 May 2010 14:43:16 +0300
  
  IOADDRESS type definition encompass more than just pci code, so move it to
  common.
  
  diff --git a/hw/xfree86/os-support/bus/xf86Pci.h 
  b/hw/xfree86/os-support/bus/xf86Pci.h
  index ce1336b..a9a2779 100644
  --- a/hw/xfree86/os-support/bus/xf86Pci.h
  +++ b/hw/xfree86/os-support/bus/xf86Pci.h
  @@ -235,7 +235,6 @@
   
   /* Primitive Types */
   typedef unsigned long ADDRESS; /* Memory/PCI address */
  -typedef unsigned long IOADDRESS;   /* Must be large enough for a pointer */
   typedef unsigned long PCITAG;
 
 Moving IOADDRESS over, but leaving the other ibviously related ones
 behind, seems a bad idea to me.

All right, I'll do it.

But honestly I've done this initial patch a bit against my will because all
this typedefs should be only constrained inside PCI code, but with the current
code base seems really tough :(

Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH resent] configure: introduce --{enable, disable}-fontserver

[Vignatti Tiago (Nokia-D/Helsinki)]

On Fri, May 21, 2010 at 04:54:01PM +0200, ext Keith Packard wrote:
 On Wed,  5 May 2010 17:14:41 +0300, Tiago Vignatti tiago.vigna...@nokia.com 
 wrote:
 
  Keith, no one took this one. Care to pull it?
 
 I though I had replied suggesting that this should depend on what support was
 provided in the system libXfont instead of having the server be
 configured separately.

yeah, adjust .pc from libXfont seems to solve the issue. I'm just lazily
hanging this work for some days because I don't know exactly the trick to do
so.


Maybe some autoconf ninja on the list knows?

Tiago
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: clang static analysis

[Jamey Sharp]

On Thu, May 20, 2010 at 11:59 PM, Jeremy Huddleston jerem...@apple.com wrote:
 In case anyone's interested, I've posted an updated log from building the 
 XQuartz server with clang SA:

 http://people.freedesktop.org/~jeremyhu/clang/2010-05-20-1/

The first one I examined (Assigned value is garbage or undefined,
mi/miwideline.c line 1918) assumes in steps 10 and 11 that
pGC-lineStyle == LineOnOffDash and pGC-capStyle == CapProjecting,
but took the false branch for that condition at step 8. Is it often
that wrong? (It makes the same mistake for line 1804, and the Branch
condition evaluates to a garbage value error is based on a similarly
contradictory assumption.) A tiny bit of abstract interpretation would
go a long way here...

Assigned value is garbage or undefined at dix/dixfonts.c line 1231
is correct, at least, except the real bug is that I didn't delete the
oldfid variable when I deleted the code that cared what value it had.
Patch for that shortly.

I suspect quite a few dead-store warnings would go away if we kill the
REGION_* macros. One particularly entertaining example, from
http://people.freedesktop.org/~jeremyhu/clang/2010-05-20-1/report-7FMXPb.html

/* This prevents warnings about pScreen not being used. */
pGC-pScreen = pScreen = pGC-pScreen;

clang says Although the value stored to 'pScreen' is used in the
enclosing expression, the value is never actually read from
'pScreen'.

Jamey
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 04/11] xfree86: bus: remove superfluous and confused structures in BusRec

[Keith Packard]

On Fri, 21 May 2010 19:50:56 +0300, Vignatti Tiago (Nokia-D/Helsinki) 
tiago.vigna...@nokia.com wrote:

 Did I change your opinion? :)

No, brevity is not always a compelling argument. In this case, I'd
rather have clarity.

-- 
keith.pack...@intel.com


pgpJOvAu4Vzly.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/2] xfree86: vgaarb: simplify the arguments passed to lock/unlock

[Vignatti Tiago (Nokia-D/Helsinki)]

On Fri, May 21, 2010 at 06:43:11PM +0200, ext Keith Packard wrote:
 On Fri, 21 May 2010 18:48:14 +0300, Tiago Vignatti tiago.vigna...@nokia.com 
 wrote:
 
  Send only screen index instead the whole rec for lock and remove the 
  argument
  of unlock.
 
 nak -- the arbiter presumably needs to run during server initialization
 before the screen is allocated, which means that any per-GPU data will
 end up hanging from the scrninfo in any case.
 
You gave the nak for the wrong patch from the series, right? First patch is 
only a cleanup.

Anyway, as I said in the second patch, I'm pretty sure we can allocate
ScreenRec before to make this idea succeeds. I'll think about it and come
back with something.


Tiago
~   
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] doPolyText: forget about FontChange's XID after looking up pFont.

[Jamey Sharp]

As of e2929db7b737413cf93fbebdf4d15abdfebff05c, doPolyText uses pFont
consistently rather than looking it up again from the saved XID.

clang noticed that fid = oldfid could run when oldfid hadn't been
initialized yet.

Signed-off-by: Jamey Sharp ja...@minilop.net
---
 dix/dixfonts.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/dix/dixfonts.c b/dix/dixfonts.c
index ba1d0e2..b6d54f8 100644
--- a/dix/dixfonts.c
+++ b/dix/dixfonts.c
@@ -1173,7 +1173,6 @@ int
 doPolyText(ClientPtr client, PTclosurePtr c)
 {
 FontPtr pFont = c-pGC-font, oldpFont;
-Font   fid, oldfid;
 int err = Success, lgerr;  /* err is in X error, not font error, space */
 enum { NEVER_SLEPT, START_SLEEP, SLEEPING } client_state = NEVER_SLEPT;
 FontPathElementPtr fpe;
@@ -1221,6 +1220,7 @@ doPolyText(ClientPtr client, PTclosurePtr c)
 {
if (*c-pElt == FontChange)
 {
+   Font fid;
if (c-endReq - c-pElt  FontShiftSize)
{
 err = BadLength;
@@ -1228,7 +1228,6 @@ doPolyText(ClientPtr client, PTclosurePtr c)
}
 
oldpFont = pFont;
-   oldfid = fid;
 
fid =  ((Font)*(c-pElt+4)) /* big-endian */
 | ((Font)*(c-pElt+3))  8
@@ -1238,9 +1237,8 @@ doPolyText(ClientPtr client, PTclosurePtr c)
  client, DixUseAccess);
if (err != Success)
{
-   /* restore pFont and fid for step 4 (described below) */
+   /* restore pFont for step 4 (described below) */
pFont = oldpFont;
-   fid = oldfid;
 
/* If we're in START_SLEEP mode, the following step
   shortens the request...  in the unlikely event that
-- 
1.7.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] vfb: Remove dead variable and header file

[Adam Jackson]

On Mon, 2010-05-10 at 11:48 -0400, Adam Jackson wrote:
 Signed-off-by: Adam Jackson a...@redhat.com

Resend, trivial dead code deletion.

- ajax


signature.asc
Description: This is a digitally signed message part
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] vfb: Remove dead variable and header file

[Jamey Sharp]

On Fri, May 21, 2010 at 11:07 AM, Adam Jackson a...@nwnk.net wrote:
 On Mon, 2010-05-10 at 11:48 -0400, Adam Jackson wrote:
 Signed-off-by: Adam Jackson a...@redhat.com

 Resend, trivial dead code deletion.

Which I even provided a reviewed-by tag for. Keith?

Jamey
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] doPolyText: forget about FontChange's XID after looking up pFont.

[Jamey Sharp]

As of e2929db7b737413cf93fbebdf4d15abdfebff05c, doPolyText uses pFont
consistently rather than looking it up again from the saved XID.

clang noticed that oldfid = fid could run when fid hadn't been
initialized yet.

Signed-off-by: Jamey Sharp ja...@minilop.net
---
Same patch, with a commit message that isn't wrong this time.

 dix/dixfonts.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/dix/dixfonts.c b/dix/dixfonts.c
index ba1d0e2..b6d54f8 100644
--- a/dix/dixfonts.c
+++ b/dix/dixfonts.c
@@ -1173,7 +1173,6 @@ int
 doPolyText(ClientPtr client, PTclosurePtr c)
 {
 FontPtr pFont = c-pGC-font, oldpFont;
-Font   fid, oldfid;
 int err = Success, lgerr;  /* err is in X error, not font error, space */
 enum { NEVER_SLEPT, START_SLEEP, SLEEPING } client_state = NEVER_SLEPT;
 FontPathElementPtr fpe;
@@ -1221,6 +1220,7 @@ doPolyText(ClientPtr client, PTclosurePtr c)
 {
if (*c-pElt == FontChange)
 {
+   Font fid;
if (c-endReq - c-pElt  FontShiftSize)
{
 err = BadLength;
@@ -1228,7 +1228,6 @@ doPolyText(ClientPtr client, PTclosurePtr c)
}
 
oldpFont = pFont;
-   oldfid = fid;
 
fid =  ((Font)*(c-pElt+4)) /* big-endian */
 | ((Font)*(c-pElt+3))  8
@@ -1238,9 +1237,8 @@ doPolyText(ClientPtr client, PTclosurePtr c)
  client, DixUseAccess);
if (err != Success)
{
-   /* restore pFont and fid for step 4 (described below) */
+   /* restore pFont for step 4 (described below) */
pFont = oldpFont;
-   fid = oldfid;
 
/* If we're in START_SLEEP mode, the following step
   shortens the request...  in the unlikely event that
-- 
1.7.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: clang static analysis

[Jeremy Huddleston]

The analyzer is correct.  It sees the call to miFillPolyHelper on line 1849 and 
assumes that pGC can change:

miFillPolyHelper (pDrawable, pGC, pixel, spanData, y, h, left, right, nleft, 
nright);

because it is not const:

static void
miFillPolyHelper (DrawablePtr pDrawable, GCPtr pGC, unsigned long pixel,
  SpanDataPtr spanData, int y, int overall_height,
  PolyEdgePtr left, PolyEdgePtr right,
  int left_count, int right_count)

My guess is that applying const correctly in many places will help the SA 
avoid false positives like this.


On May 21, 2010, at 10:32, Jamey Sharp wrote:

 On Thu, May 20, 2010 at 11:59 PM, Jeremy Huddleston jerem...@apple.com 
 wrote:
 In case anyone's interested, I've posted an updated log from building the 
 XQuartz server with clang SA:
 
 http://people.freedesktop.org/~jeremyhu/clang/2010-05-20-1/
 
 The first one I examined (Assigned value is garbage or undefined,
 mi/miwideline.c line 1918) assumes in steps 10 and 11 that
 pGC-lineStyle == LineOnOffDash and pGC-capStyle == CapProjecting,
 but took the false branch for that condition at step 8. Is it often
 that wrong? (It makes the same mistake for line 1804, and the Branch
 condition evaluates to a garbage value error is based on a similarly
 contradictory assumption.) A tiny bit of abstract interpretation would
 go a long way here...
 
 Assigned value is garbage or undefined at dix/dixfonts.c line 1231
 is correct, at least, except the real bug is that I didn't delete the
 oldfid variable when I deleted the code that cared what value it had.
 Patch for that shortly.
 
 I suspect quite a few dead-store warnings would go away if we kill the
 REGION_* macros. One particularly entertaining example, from
 http://people.freedesktop.org/~jeremyhu/clang/2010-05-20-1/report-7FMXPb.html
 
/* This prevents warnings about pScreen not being used. */
pGC-pScreen = pScreen = pGC-pScreen;
   
 clang says Although the value stored to 'pScreen' is used in the
 enclosing expression, the value is never actually read from
 'pScreen'.
 
 Jamey

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] doPolyText: forget about FontChange's XID after looking up pFont.

[Jeremy Huddleston]

Reviewed-by: Jeremy Huddleston jerem...@apple.com
Tested-by: Jeremy Huddleston jerem...@apple.com

On May 21, 2010, at 11:12, Jamey Sharp wrote:

 As of e2929db7b737413cf93fbebdf4d15abdfebff05c, doPolyText uses pFont
 consistently rather than looking it up again from the saved XID.
 
 clang noticed that oldfid = fid could run when fid hadn't been
 initialized yet.
 
 Signed-off-by: Jamey Sharp ja...@minilop.net
 ---
 Same patch, with a commit message that isn't wrong this time.
 
 dix/dixfonts.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/dix/dixfonts.c b/dix/dixfonts.c
 index ba1d0e2..b6d54f8 100644
 --- a/dix/dixfonts.c
 +++ b/dix/dixfonts.c
 @@ -1173,7 +1173,6 @@ int
 doPolyText(ClientPtr client, PTclosurePtr c)
 {
 FontPtr pFont = c-pGC-font, oldpFont;
 -Font fid, oldfid;
 int err = Success, lgerr; /* err is in X error, not font error, space */
 enum { NEVER_SLEPT, START_SLEEP, SLEEPING } client_state = NEVER_SLEPT;
 FontPathElementPtr fpe;
 @@ -1221,6 +1220,7 @@ doPolyText(ClientPtr client, PTclosurePtr c)
 {
   if (*c-pElt == FontChange)
 {
 + Font fid;
   if (c-endReq - c-pElt  FontShiftSize)
   {
err = BadLength;
 @@ -1228,7 +1228,6 @@ doPolyText(ClientPtr client, PTclosurePtr c)
   }
 
   oldpFont = pFont;
 - oldfid = fid;
 
   fid =  ((Font)*(c-pElt+4)) /* big-endian */
| ((Font)*(c-pElt+3))  8
 @@ -1238,9 +1237,8 @@ doPolyText(ClientPtr client, PTclosurePtr c)
 client, DixUseAccess);
   if (err != Success)
   {
 - /* restore pFont and fid for step 4 (described below) */
 + /* restore pFont for step 4 (described below) */
   pFont = oldpFont;
 - fid = oldfid;
 
   /* If we're in START_SLEEP mode, the following step
  shortens the request...  in the unlikely event that
 -- 
 1.7.0
 
 ___
 xorg-devel@lists.x.org: X.Org development
 Archives: http://lists.x.org/archives/xorg-devel
 Info: http://lists.x.org/mailman/listinfo/xorg-devel

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 0/4] Change region APIs

[Keith Packard]

This patch sequence starts out by moving the region implementation
from mi to dix, changing the names of all of the functions to
eliminate the leading mi. This patch includes a sed script which was
used to mechanically change all of the files, the only manual edits
were to move the region.c file and fix up the Makefiles.

Next, I wrote another sed script to mechanically convert all of the
region macros so that they no longer take a screen argument. I renamed
them to conform to the standard StudlyCaps server conventions largely
to make it possible to preserve source compatibility by providing the
old names as wrappers.

Third, I rewrote the region implementation from macros to static
inline functions and to replace the functions which were simply
wrapping pixman region functions with static inlines that called those
directly.

Finally, I added all of the old names for source code compatibility
with existing drivers. This is a separate patch so that I could ensure
no code remained in the server using the old names.

Keith Packard (4):
  Move region implementation from mi to dix.
  Rename region macros to mixed case and remove screen argument
  Change region implementation from macros to inline functions.
  Add REGION_ macros for source compatibility with existing drivers.

 Xext/panoramiX.c |   34 +-
 Xext/panoramiXprocs.c|   24 +-
 Xext/shape.c |   48 +-
 Xext/xace.c  |   14 +-
 composite/compalloc.c|6 +-
 composite/compext.c  |2 +-
 composite/compwindow.c   |   42 +-
 damageext/damageext.c|   12 +-
 dix/Makefile.am  |1 +
 dix/dispatch.c   |   12 +-
 dix/events.c |   60 +-
 dix/region.c | 1685 ++
 dix/window.c |  104 ++--
 doc/xml/Xserver-spec.xml |   40 +-
 exa/exa.c|6 +-
 exa/exa_accel.c  |   80 +-
 exa/exa_classic.c|8 +-
 exa/exa_migration_classic.c  |   60 +-
 exa/exa_mixed.c  |4 +-
 exa/exa_render.c |   48 +-
 exa/exa_unaccel.c|   56 +-
 fb/fb.h  |2 +-
 fb/fb24_32.c |8 +-
 fb/fbarc.c   |2 +-
 fb/fbbits.h  |4 +-
 fb/fbfill.c  |2 +-
 fb/fbfillrect.c  |6 +-
 fb/fbfillsp.c|6 +-
 fb/fbgc.c|2 +-
 fb/fbglyph.c |4 +-
 fb/fbimage.c |8 +-
 fb/fbline.c  |4 +-
 fb/fboverlay.c   |   42 +-
 fb/fbpict.c  |6 +-
 fb/fbpixmap.c|   14 +-
 fb/fbpoint.c |2 +-
 fb/fbpush.c  |4 +-
 fb/fbseg.c   |4 +-
 fb/fbsetsp.c |4 +-
 fb/fbwindow.c|   14 +-
 fix-miregion |   29 +
 fix-region   |   39 +
 glx/glxdri.c |   12 +-
 glx/glxdri2.c|6 +-
 hw/dmx/dmxextension.c|4 +-
 hw/dmx/dmxgc.c   |4 +-
 hw/dmx/dmxgcops.c|   18 +-
 hw/dmx/dmxpict.c |4 +-
 hw/dmx/dmxpixmap.c   |   18 +-
 hw/dmx/dmxshadow.c   |4 +-
 hw/dmx/dmxwindow.c   |8 +-
 hw/dmx/input/dmxconsole.c|2 +-
 hw/kdrive/ephyr/ephyr.c  |   12 +-
 hw/kdrive/ephyr/ephyrdriext.c|   18 +-
 hw/kdrive/ephyr/ephyrvideo.c |   16 +-
 hw/kdrive/src/kdrive.c   |   14 +-
 hw/kdrive/src/kxv.c  |  158 ++--
 hw/xfree86/common/xf86Helper.c   |   16 +-
 hw/xfree86/common/xf86fbman.c|  132 ++--
 hw/xfree86/common/xf86xv.c   |  172 ++--
 hw/xfree86/dri/dri.c |   36 +-
 hw/xfree86/dri2/dri2.c   |8 +-
 hw/xfree86/modes/xf86Crtc.c  |6 +-
 hw/xfree86/modes/xf86Rotate.c|   18 +-
 hw/xfree86/shadowfb/shadow.c |   34 +-
 hw/xfree86/xaa/xaaBitBlt.c   |   34 +-
 hw/xfree86/xaa/xaaCpyArea.c  |   12 +-
 hw/xfree86/xaa/xaaCpyPlane.c |   12 +-
 hw/xfree86/xaa/xaaCpyWin.c   |   14 +-
 hw/xfree86/xaa/xaaDashLine.c |4 +-
 hw/xfree86/xaa/xaaFillArc.c  |4 +-
 hw/xfree86/xaa/xaaFillPoly.c |   12 +-
 hw/xfree86/xaa/xaaFillRect.c |   22 +-
 hw/xfree86/xaa/xaaImage.c|4 +-
 hw/xfree86/xaa/xaaLine.c |4 +-
 hw/xfree86/xaa/xaaNonTEText.c|   20 +-
 hw/xfree86/xaa/xaaOverlay.c  |   16 +-
 hw/xfree86/xaa/xaaOverlayDF.c|   28 +-
 hw/xfree86/xaa/xaaPCache.c   |4 +-
 hw/xfree86/xaa/xaaPict.c |   40 +-
 hw/xfree86/xaa/xaaRect.c |4 +-
 hw/xfree86/xaa/xaaSpans.c|   10 +-
 hw/xfree86/xaa/xaaTEText.c   |   12 +-
 

[PATCH 4/4] Add REGION_ macros for source compatibility with existing drivers.

[Keith Packard]

This makes the region code changes source compatible with existing
code, although none of them are used within the server source itself.

Signed-off-by: Keith Packard kei...@keithp.com
---
 include/regionstr.h |   39 +++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/include/regionstr.h b/include/regionstr.h
index a670370..bc8d88e 100644
--- a/include/regionstr.h
+++ b/include/regionstr.h
@@ -328,4 +328,43 @@ extern _X_EXPORT int ClipSpans(
 int /*fSorted*/
 );
 
+#define INCLUDE_LEGACY_REGION_DEFINES
+#ifdef INCLUDE_LEGACY_REGION_DEFINES
+
+#define REGION_NIL RegionNil
+#define REGION_NAR RegionNar
+#define REGION_NUM_RECTS   RegionNumRects
+#define REGION_SIZERegionSize
+#define REGION_RECTS   RegionRects
+#define REGION_BOXPTR  RegionBoxptr
+#define REGION_BOX RegionBox
+#define REGION_TOP RegionTop
+#define REGION_END RegionEnd
+#define REGION_SZOFRegionSizeof
+#define BITMAP_TO_REGION   BitmapToRegion
+#define REGION_CREATE(pScreen, r, s)   RegionCreate(r,s)
+#define REGION_COPY(pScreen, d, r) RegionCopy(d, r)
+#define REGION_DESTROY(pScreen, r) RegionDestroy(r)
+#define REGION_INTERSECT(pScreen, res, r1, r2) RegionIntersect(res, r1, r2)
+#define REGION_UNION(pScreen, res, r1, r2) RegionUnion(res, r1, r2)
+#define REGION_SUBTRACT(pScreen, res, r1, r2)  RegionSubtract(res, r1, r2)
+#define REGION_INVERSE(pScreen, n, r, b)   RegionInverse(n, r, b)
+#define REGION_TRANSLATE(pScreen, r, x, y) RegionTranslate(r, x, y)
+#define RECT_IN_REGION(pScreen, r, b)  RectInRegion(r, b)
+#define POINT_IN_REGION(pScreen, r, x, y, b)   PointInRegion(r, x, y, b)
+#define REGION_EQUAL(pScreen, r1, r2)  RegionEqual(r1, r2)
+#define REGION_APPEND(pScreen, d, r)   RegionAppend(d, r)
+#define REGION_VALIDATE(pScreen, r, o) RegionValidate(r, o)
+#define RECTS_TO_REGION(pScreen, n, r, c)  RectsToRegion(n, r, c)
+#define REGION_BREAK(pScreen, r)   RegionBreak(r)
+#define REGION_INIT(pScreen, r, b, s)  RegionInit(r, b, s)
+#define REGION_UNINIT(pScreen, r)  RegionUninit(r)
+#define REGION_RESET(pScreen, r, b)RegionReset(r, b)
+#define REGION_NOTEMPTY(pScreen, r)RegionNotEmpty(r)
+#define REGION_BROKEN(pScreen, r)  RegionBroken(r)
+#define REGION_EMPTY(pScreen, r)   RegionEmpty(r)
+#define REGION_EXTENTS(pScreen, r) RegionExtents(r)
+#define REGION_NULL(pScreen, r)RegionNull(r)
+
+#endif /* INCLUDE_LEGACY_REGION_DEFINES */
 #endif /* REGIONSTRUCT_H */
-- 
1.7.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/2] xfree86: vgaarb: simplify the arguments passed to lock/unlock

[Keith Packard]

On Fri, 21 May 2010 20:49:06 +0300, Vignatti Tiago (Nokia-D/Helsinki) 
tiago.vigna...@nokia.com wrote:

 You gave the nak for the wrong patch from the series, right? First patch is 
 only a cleanup.

No, it's not a cleanup, it's an API change which passes the job of
computing the argument into the lower level function. Using a nice typed
pointer is safer than passing a bare int. The motivation for this
change was to change the implementation to use the Screen instead of the
ScrnInt, although I'm not sure why you want to make this change.

-- 
keith.pack...@intel.com


pgpqYQBJaj2E8.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] vfb: Remove dead variable and header file

[Keith Packard]

On Fri, 21 May 2010 11:10:20 -0700, Jamey Sharp ja...@minilop.net wrote:
 On Fri, May 21, 2010 at 11:07 AM, Adam Jackson a...@nwnk.net wrote:
  On Mon, 2010-05-10 at 11:48 -0400, Adam Jackson wrote:
  Signed-off-by: Adam Jackson a...@redhat.com
 
  Resend, trivial dead code deletion.
 
 Which I even provided a reviewed-by tag for. Keith?

Right, not in my queue as it wasn't cc'd to me. I'm trying to scan
xorg-devel to make sure stuff doesn't get lost, but having my name on it
makes it easier to keep track of.

-- 
keith.pack...@intel.com


pgpBh51wzkzwr.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/4] Move region implementation from mi to dix.

[Jamey Sharp]

It looks like renaming miSubtractSpans to SubtractSpans was an accident?

With that fixed, this looks perfectly sensible.

Reviewed-by: Jamey Sharp ja...@minilop.net

Jamey
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/4] Move region implementation from mi to dix.

[Keith Packard]

On Fri, 21 May 2010 13:27:20 -0700, Jamey Sharp ja...@minilop.net wrote:

 It looks like renaming miSubtractSpans to SubtractSpans was an accident?
 
 With that fixed, this looks perfectly sensible.

Yeah, the sed script was a bit incautious. I've fixed the sed script and
regenerated the branch.

In particular, I've split the huge patch into two pieces, the first is
purely mechanical and done with the fix-region sed-script, the second
patch is the needed fix-ups to make it actually work. When merging this
to the server, I would smash those two together to make it bi-sectable,
but for review I've split them apart to make it easier to see the two
steps.

I'll merge the mi - dix change in later.

-- 
keith.pack...@intel.com


pgpU3MyPWoajc.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 2/4] Rename region macros to mixed case and remove screen argument

[Keith Packard]

On Fri, 21 May 2010 15:12:47 -0700, Jamey Sharp ja...@minilop.net wrote:

 I get different results from running fix-region than are reflected in
 this patch. I think the script is giving the right output, and the
 patch is wrong in places.

I think that's fixed in the new push I just made; are you looking at
that one?

 - doc/xml/Xserver-spec.xml incorrectly dropped the pScreen argument
 from BitmapToRegion.
 - The patch misses some REGION_BREAK uses.
 - Most of the RegionDestroy calls wound up with a space after the open-paren.

Ok, these are all fixed in the new push I just did.


 It's kind of confusing having some functions, like RegionInit, get
 disabled with #ifndef in this patch. I know the next commit cleans
 that up, but since you don't use the code from the function
 implementations at all in that later patch, can I suggest just
 deleting them in this one?

I think having this patch be shorter makes it easier to see what
happened?

 This hardly should block merge, but would you consider fixing the `git
 diff --check` whitespace warnings in this patch? None of them were
 introduced here (except in the sed script) but it'd be nice to fix
 them as long as you're touching these lines anyway.

Sigh. Is there an easy way to do this that doesn't touch every line in
the server?

-- 
keith.pack...@intel.com


pgpXR6pDEQBwK.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/4] Move region implementation from mi to dix.

[Jamey Sharp]

On Fri, May 21, 2010 at 3:14 PM, Keith Packard kei...@keithp.com wrote:
 On Fri, 21 May 2010 13:27:20 -0700, Jamey Sharp ja...@minilop.net wrote:
 It looks like renaming miSubtractSpans to SubtractSpans was an accident?

 With that fixed, this looks perfectly sensible.

 Yeah, the sed script was a bit incautious. I've fixed the sed script and
 regenerated the branch.

You even incorporated a change I hadn't finished writing an e-mail to
suggest. Strong work. :-)

Seems like the deletions of mi prefixes in region.c should still go
in this first commit, though, not in Change region implementation
from macros to inline functions. The latter looks kind of broken now.

Also, it looks like you ran fix-miregion over fix-miregion. Oops.

 In particular, I've split the huge patch into two pieces, the first is
 purely mechanical and done with the fix-region sed-script, the second
 patch is the needed fix-ups to make it actually work. When merging this
 to the server, I would smash those two together to make it bi-sectable,
 but for review I've split them apart to make it easier to see the two
 steps.

Good plan. That does help.

Jamey
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/4] Move region implementation from mi to dix.

[Keith Packard]

On Fri, 21 May 2010 15:36:07 -0700, Jamey Sharp ja...@minilop.net wrote:

 Seems like the deletions of mi prefixes in region.c should still go
 in this first commit, though, not in Change region implementation
 from macros to inline functions. The latter looks kind of broken now.

Yeah, you're right; deleting the code in the first patch makes things
clearer later on.

-- 
keith.pack...@intel.com


pgpLY8v49k1Yw.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 2/4] Rename region macros to mixed case and remove screen argument

[Jamey Sharp]

On Fri, May 21, 2010 at 3:32 PM, Keith Packard kei...@keithp.com wrote:
 On Fri, 21 May 2010 15:12:47 -0700, Jamey Sharp ja...@minilop.net wrote:
 I get different results from running fix-region than are reflected in
 this patch. I think the script is giving the right output, and the
 patch is wrong in places.

 I think that's fixed in the new push I just made; are you looking at
 that one?

I wasn't, and it is indeed fixed.

 It's kind of confusing having some functions, like RegionInit, get
 disabled with #ifndef in this patch. I know the next commit cleans
 that up, but since you don't use the code from the function
 implementations at all in that later patch, can I suggest just
 deleting them in this one?

 I think having this patch be shorter makes it easier to see what
 happened?

I have a lot of sympathy for the smaller diff argument, but if it
were me, I'd still delete them...

 This hardly should block merge, but would you consider fixing the `git
 diff --check` whitespace warnings in this patch? None of them were
 introduced here (except in the sed script) but it'd be nice to fix
 them as long as you're touching these lines anyway.

 Sigh. Is there an easy way to do this that doesn't touch every line in
 the server?

This seems to work:

git diff --check |
sed -n 's!^\([^:]*\):\([^:]*\):.*!sed -i \2 s/[ \t]*$//; \2 s/
*\t/\t/g \1!p' |
sh

I'm inordinately pleased at using sed to generate sed scripts.

Jamey
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 2/4] Rename region macros to mixed case and remove screen argument

[Keith Packard]

On Fri, 21 May 2010 15:57:47 -0700, Jamey Sharp ja...@minilop.net wrote:

 This seems to work:
 
 git diff --check |
 sed -n 's!^\([^:]*\):\([^:]*\):.*!sed -i \2 s/[ \t]*$//; \2 s/
 *\t/\t/g \1!p' |
 sh
 
 I'm inordinately pleased at using sed to generate sed scripts.

Very cool. Pushed.

-- 
keith.pack...@intel.com


pgpoVmyI6izUr.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 2/4] Rename region macros to mixed case and remove screen argument

[Jamey Sharp]

On Fri, May 21, 2010 at 5:18 PM, Keith Packard kei...@keithp.com wrote:
 Very cool. Pushed.

I have one thing I'm still confused about before I give my reviewed-by
for the series. Change region implementation from macros to inline
functions has an assortment of renamings in region.c to remove the
mi prefix from various names. Most of those are visible in
regionstr.h. Did you intend to defer those that late or did you want
to get them moved back to Move region implementation from mi to dix,
where those changes were in the first place?

I'm pretty sure that renaming miValidateTree to ValidateTree in a
region.c comment was just an accident, but I'm less convinced about
your intent for the rest.

Jamey
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/4] Move region implementation from mi to dix.

[Soeren Sandmann]

Keith Packard kei...@keithp.com writes:

 There isn't any reason to have this in mi. Everything except the file
 rename was done with the included script 'fix-miregion'.

Would it be useful to split this commit into two: one that just
renames the file and one that is the result of running the script?

That way it becomes easier to see what precisely the script actually
did, though it does mean there is an intermediate step where the
region functions have the wrong name.

Also, if we are renaming things anyway, maybe some functions should
get moved into the Region name space. For example PointInRegion()
could become RegionContainsPoint().

Other than those things, it looks good to me.


Søren
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/4] Move region implementation from mi to dix.

[Soeren Sandmann]

Keith Packard kei...@keithp.com writes:

 There isn't any reason to have this in mi. Everything except the file
 rename was done with the included script 'fix-miregion'.

Didn't realize that the hand-rolled changes commit used to be part
of this. The comment I have on that part is that it seems to preserve
the macro versions of some ops rather than the function versions. For
example, I think it would be better to have this:

 void
 miRegionInit(RegionPtr pReg, BoxPtr rect, int size)
 {
 if (rect)
pixman_region_init_with_extents (pReg, rect);
 else
pixman_region_init (pReg);
 }

as an inline function rather than the version where those two pixman
functions are manually inlined.

Similarly, these functions

   RegionUninit
   RegionReset
   RegionNotEmpty
   RegionEmpty  - !RegionNotEmpty

could just call the pixman equivalent like most of the other Region*
API.


Søren
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/4] Move region implementation from mi to dix.

[Soeren Sandmann]

Keith Packard kei...@keithp.com writes:

 There isn't any reason to have this in mi. Everything except the file
 rename was done with the included script 'fix-miregion'.

There is a search-and-replace bug here:

- * to miValidateTree.  Bob Scheifler changed the representation to be more
+ * to ValidateTree.  Bob Scheifler changed the representation to be more

But miValidateTree still exists.

In your branch, this happens in the 

Change region implementation from macros to inline functions

commit.


Søren
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: Yet another reason window pictures suck

[Soeren Sandmann]

Chris Wilson ch...@chris-wilson.co.uk writes:

 On Fri, 21 May 2010 11:44:39 -0400, Adam Jackson a...@redhat.com wrote:
  Context:
  
  https://bugzilla.redhat.com/show_bug.cgi?id=533879
  
  If you've got a 24bpp framebuffer, you poor misguided soul you, then you
  have a conundrum.  Pixmaps are 32bpp but Windows aren't.  Suppose you
  make a Picture out of a Window.  You would think you'd be required to
  use one of the Formats with an associated Visual.  What does the spec
  say about it?
  
 [...]
  
  b) Fix the spec to be more strict, and fix cairo and whoever else to be
  more careful about picking Visual Formats for Window Pictures.
 
 The bad news here is that the XRenderFormat chosen (for
 cairo_xlib_surface_create_with_xrender_format()) is entirely the
 responsibility of the application. Visuals are mapped to XRenderFormats
 using the mapping supplied by RenderQueryPictFormats. Cairo itself
 only creates [ax]rgb32 (and a8) pixmaps. So we would need to teach the
 toolkits to handle 24bpp Windows.
 
 The implementation should be fixed though, let's return BadMatch and
 stop this silent corruption.

Is it actually silent corruption? pixman will explode spectactularly
if you give it a format that it doesn't know about.

Isn't the problem just that render advertises a bogus format that some
applications will then make use of?


Søren

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] Clean up after removal of screen parameters from region macros.

[Jamey Sharp]

Signed-off-by: Jamey Sharp ja...@minilop.net
---
This patch depends on Keith's change-region-api series.

Jeremy, I'd be curious to see a clang run with this series applied,
since that was the immediate motivation for it.

After hand-inspecting about 1,500 references to the region functions,
I'm not going to be excited about bikeshedding. Please, just don't. :-)

 Xext/panoramiXprocs.c|   13 --
 Xext/shape.c |   14 ++-
 Xext/xace.c  |3 --
 damageext/damageext.c|2 -
 dix/dispatch.c   |   13 --
 dix/events.c |   38 ---
 dix/window.c |   46 -
 fb/fb.h  |3 +-
 fb/fbglyph.c |2 -
 fb/fboverlay.c   |   28 +--
 hw/dmx/dmxpixmap.c   |3 +-
 hw/xfree86/common/xf86xv.c   |   21 -
 hw/xfree86/shadowfb/shadow.c |   26 -
 hw/xnest/Window.c|   18 +--
 hw/xwin/winmultiwindowshape.c|5 +---
 hw/xwin/winwindow.c  |1 -
 hw/xwin/winwindowswm.c   |2 -
 mi/mi.h  |1 -
 mi/micopy.c  |3 +-
 mi/miexpose.c|4 ---
 mi/migc.c|8 +-
 mi/mioverlay.c   |   26 +++--
 mi/mivaltree.c   |   14 +--
 mi/miwindow.c|   15 +---
 miext/damage/damage.c|3 +-
 miext/rootless/rootlessCommon.c  |7 +++--
 miext/rootless/rootlessValTree.c |   19 +--
 miext/rootless/rootlessWindow.c  |   15 +--
 render/mirect.c  |3 +-
 render/picture.c |8 ++
 xfixes/region.c  |4 +--
 31 files changed, 105 insertions(+), 263 deletions(-)

diff --git a/Xext/panoramiXprocs.c b/Xext/panoramiXprocs.c
index 9b39b1a..05c40fe 100644
--- a/Xext/panoramiXprocs.c
+++ b/Xext/panoramiXprocs.c
@@ -620,10 +620,9 @@ int PanoramiXTranslateCoords(ClientPtr client)
 * borderSize
 */
 (!wBoundingShape(pWin) ||
-   PointInRegion(
-   wBoundingShape(pWin), 
-   x - pWin-drawable.x, 
-   y - pWin-drawable.y, box))
+   PointInRegion(wBoundingShape(pWin),
+ x - pWin-drawable.x,
+ y - pWin-drawable.y, box))
)
 {
rep.child = pWin-drawable.id;
@@ -1128,7 +1127,6 @@ int PanoramiXCopyArea(ClientPtr client)
}
 
if(pGC-graphicsExposures) {
-   ScreenPtr pScreen = pDst-pScreen;
RegionRec totalReg;
Bool overlap;
 
@@ -1144,7 +1142,7 @@ int PanoramiXCopyArea(ClientPtr client)
}
}
RegionValidate(totalReg, overlap);
-   (*pScreen-SendGraphicsExpose)(
+   (*pDst-pScreen-SendGraphicsExpose)(
client, totalReg, stuff-dstDrawable, X_CopyArea, 0);
RegionUninit(totalReg);
}
@@ -1243,7 +1241,6 @@ int PanoramiXCopyPlane(ClientPtr client)
 }
 
 if(pGC-graphicsExposures) {
-   ScreenPtr pScreen = pdstDraw-pScreen;
RegionRec totalReg;
Bool overlap;
 
@@ -1255,7 +1252,7 @@ int PanoramiXCopyPlane(ClientPtr client)
}
}
RegionValidate(totalReg, overlap);
-   (*pScreen-SendGraphicsExpose)(
+   (*pdstDraw-pScreen-SendGraphicsExpose)(
client, totalReg, stuff-dstDrawable, X_CopyPlane, 0);
RegionUninit(totalReg);
 }
diff --git a/Xext/shape.c b/Xext/shape.c
index afa1d8f..f7b20b2 100644
--- a/Xext/shape.c
+++ b/Xext/shape.c
@@ -150,8 +150,6 @@ RegionOperate (
int xoff, int yoff,
CreateDftPtr create)
 {
-ScreenPtr  pScreen = pWin-drawable.pScreen;
-
 if (srcRgn  (xoff || yoff))
RegionTranslate(srcRgn, xoff, yoff);
 if (!pWin-parent)
@@ -220,7 +218,7 @@ RegionOperate (
 }
 if (srcRgn)
RegionDestroy(srcRgn);
-(*pScreen-SetShape) (pWin);
+(*pWin-drawable.pScreen-SetShape) (pWin);
 SendShapeNotify (pWin, kind);
 return Success;
 }
@@ -281,7 +279,6 @@ static int
 ProcShapeRectangles (ClientPtr client)
 {
 WindowPtr  pWin;
-ScreenPtr  pScreen;
 REQUEST(xShapeRectanglesReq);
 xRectangle *prects;
 intnrects, ctype, rc;
@@ -314,7 +311,6 @@ ProcShapeRectangles (ClientPtr client)
client-errorValue = stuff-ordering;
 return BadValue;
 }
-pScreen = pWin-drawable.pScreen;
 nrects = ((stuff-length   2) - sizeof(xShapeRectanglesReq));
 if (nrects  4)
   

[PATCH] Declare functions that unconditionally call FatalError as _X_NORETURN.

[Jamey Sharp]

For AtomError, this should fix a clang warning; in the other cases it's
just good documentation.

Signed-off-by: Jamey Sharp ja...@minilop.net
Cc: Jeremy Huddleston jerem...@apple.com
---
 include/dix.h|2 +-
 include/dixstruct.h  |2 +-
 include/extnsionst.h |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/dix.h b/include/dix.h
index 893338f..3d09bbe 100644
--- a/include/dix.h
+++ b/include/dix.h
@@ -300,7 +300,7 @@ extern _X_EXPORT Bool ValidAtom(
 extern _X_EXPORT const char *NameForAtom(
 Atom /*atom*/);
 
-extern _X_EXPORT void AtomError(void);
+extern _X_EXPORT void AtomError(void) _X_NORETURN;
 
 extern _X_EXPORT void FreeAllAtoms(void);
 
diff --git a/include/dixstruct.h b/include/dixstruct.h
index 5b1a698..9610427 100644
--- a/include/dixstruct.h
+++ b/include/dixstruct.h
@@ -53,7 +53,7 @@ typedef void (*ReplySwapPtr) (
 extern _X_EXPORT void ReplyNotSwappd (
ClientPtr   /* pClient */,
int /* size */,
-   void *  /* pbuf */);
+   void *  /* pbuf */) _X_NORETURN;
 
 typedef enum {ClientStateInitial,
  ClientStateAuthenticating,
diff --git a/include/extnsionst.h b/include/extnsionst.h
index 19c76fc..bb66dfb 100644
--- a/include/extnsionst.h
+++ b/include/extnsionst.h
@@ -84,7 +84,7 @@ extern _X_EXPORT EventSwapPtr EventSwapVector[128];
 
 extern _X_EXPORT void NotImplemented ( /* FIXME: this may move to another 
file... */
xEvent *,
-   xEvent *);
+   xEvent *) _X_NORETURN;
 
 #defineSetGCVector(pGC, VectorElement, NewRoutineAddress, Atom)\
 pGC-VectorElement = NewRoutineAddress;
-- 
1.7.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: clang static analysis

[Jamey Sharp]

On Fri, May 21, 2010 at 11:19 AM, Jeremy Huddleston jerem...@apple.com wrote:
 The analyzer is correct.  It sees the call to miFillPolyHelper on line
 1849 and assumes that pGC can change because it is not const ...

 My guess is that applying const correctly in many places will help
 the SA avoid false positives like this.

Ooh, interesting. OK. miFillPolyHelper can't take a const pGC though,
because eventually it passes it to ChangeGC (although with the invariant
that it will be changed back before returning).

What about this case though?
http://people.freedesktop.org/~jeremyhu/clang/2010-05-20-1/report-bKYbbq.html

Is clang having some kind of alias analysis trouble there? pDrawable
isn't being passed to any functions, but
((WindowPtr) pDrawable)-borderClip is...

Say, looks like this putative null-pointer dereference should be
fixable by making AtomError _X_NORETURN:
http://people.freedesktop.org/~jeremyhu/clang/2010-05-20-1/report-0TXsTP.html
I'll send that patch along shortly.

Jamey
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel