date:20100614

This reduces code duplication and fixes possible leak of d. d would leak if
allocation of Device fails.

Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com
---
 src/XGetDCtl.c |   23 +--
 1 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/XGetDCtl.c b/src/XGetDCtl.c
index 8f76a51..729b0a0 100644
--- a/src/XGetDCtl.c
+++ b/src/XGetDCtl.c
@@ -84,19 +84,15 @@ XGetDeviceControl(
 req-deviceid = dev-device_id;
 req-control = control;
 
-if (!_XReply(dpy, (xReply *)  rep, 0, xFalse)) {
-   UnlockDisplay(dpy);
-   SyncHandle();
-   return (XDeviceControl *) NULL;
-}
+if (!_XReply(dpy, (xReply *)  rep, 0, xFalse))
+   goto out;
+
 if (rep.length  0) {
nbytes = (long)rep.length  2;
d = (xDeviceState *) Xmalloc((unsigned)nbytes);
if (!d) {
_XEatData(dpy, (unsigned long)nbytes);
-   UnlockDisplay(dpy);
-   SyncHandle();
-   return (XDeviceControl *) NULL;
+   goto out;
}
sav = d;
_XRead(dpy, (char *)d, nbytes);
@@ -138,11 +134,9 @@ XGetDeviceControl(
}
 
Device = (XDeviceControl *) Xmalloc((unsigned)size);
-   if (!Device) {
-   UnlockDisplay(dpy);
-   SyncHandle();
-   return (XDeviceControl *) NULL;
-   }
+   if (!Device)
+   goto out;
+
Sav = Device;
 
d = sav;
@@ -228,8 +222,9 @@ XGetDeviceControl(
default:
break;
}
-   XFree(sav);
 }
+out:
+XFree(sav);
 
 UnlockDisplay(dpy);
 SyncHandle();
-- 
1.6.3.3

___
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 1/5] libXi: Fix usage of uninitialized value

In error case length of extra data could be uninitialized. This would
result randomly sized request later in function.

Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com
---
 src/XIProperties.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/XIProperties.c b/src/XIProperties.c
index 045cc71..0f77e58 100644
--- a/src/XIProperties.c
+++ b/src/XIProperties.c
@@ -150,6 +150,7 @@ XIChangeProperty(Display* dpy, int deviceid, Atom property, 
Atom type,
 
 default:
/* BadValue will be generated */ ;
+len = 0;
 }
 
 /* we use data instead of Data32 and friends to avoid Xlib's braindead
-- 
1.6.3.3

___
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 4/5] libXi: Use single error path in XGetFeedbackControl

This reduces code duplication and fixes possible leak of f. f would leak if
allocation of Feedback fails.

Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com
---
 src/XGetFCtl.c |   23 +--
 1 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/XGetFCtl.c b/src/XGetFCtl.c
index 61df7cf..3d64404 100644
--- a/src/XGetFCtl.c
+++ b/src/XGetFCtl.c
@@ -83,20 +83,16 @@ XGetFeedbackControl(
 req-ReqType = X_GetFeedbackControl;
 req-deviceid = dev-device_id;
 
-if (!_XReply(dpy, (xReply *)  rep, 0, xFalse)) {
-   UnlockDisplay(dpy);
-   SyncHandle();
-   return (XFeedbackState *) NULL;
-}
+if (!_XReply(dpy, (xReply *)  rep, 0, xFalse))
+   goto out;
+
 if (rep.length  0) {
*num_feedbacks = rep.num_feedbacks;
nbytes = (long)rep.length  2;
f = (xFeedbackState *) Xmalloc((unsigned)nbytes);
if (!f) {
_XEatData(dpy, (unsigned long)nbytes);
-   UnlockDisplay(dpy);
-   SyncHandle();
-   return (XFeedbackState *) NULL;
+   goto out;
}
sav = f;
_XRead(dpy, (char *)f, nbytes);
@@ -134,11 +130,9 @@ XGetFeedbackControl(
}
 
Feedback = (XFeedbackState *) Xmalloc((unsigned)size);
-   if (!Feedback) {
-   UnlockDisplay(dpy);
-   SyncHandle();
-   return (XFeedbackState *) NULL;
-   }
+   if (!Feedback)
+   goto out;
+
Sav = Feedback;
 
f = sav;
@@ -253,8 +247,9 @@ XGetFeedbackControl(
f = (xFeedbackState *) ((char *)f + f-length);
Feedback = (XFeedbackState *) ((char *)Feedback + Feedback-length);
}
-   XFree((char *)sav);
 }
+out:
+XFree((char *)sav);
 
 UnlockDisplay(dpy);
 SyncHandle();
-- 
1.6.3.3

___
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/5] libXi: Fix memory leak in XIGetSelectedEvents

mask_in was leaking for every successfull XIGetSelectedEvents.

Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com
---
 src/XISelEv.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/XISelEv.c b/src/XISelEv.c
index bdc4fd1..3c1f018 100644
--- a/src/XISelEv.c
+++ b/src/XISelEv.c
@@ -161,6 +161,8 @@ XIGetSelectedEvents(Display* dpy, Window win, int 
*num_masks_return)
 
 *num_masks_return = reply.num_masks;
 
+Xfree(mask_in);
+
 return mask_out;
 
 error:
-- 
1.6.3.3

___
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 3/5] libXi: Use single error path in XQueryDeviceState

This reduces code duplication and fixes possible leak of data. data would
leak if allocation of state fails.

Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com
---
 src/XQueryDv.c |   24 +---
 1 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/src/XQueryDv.c b/src/XQueryDv.c
index d9495c2..637d5cf 100644
--- a/src/XQueryDv.c
+++ b/src/XQueryDv.c
@@ -69,7 +69,7 @@ XQueryDeviceState(
 xQueryDeviceStateReply rep;
 XDeviceState *state = NULL;
 XInputClass *any, *Any;
-char *data;
+char *data = NULL;
 XExtDisplayInfo *info = XInput_find_display(dpy);
 
 LockDisplay(dpy);
@@ -81,20 +81,15 @@ XQueryDeviceState(
 req-ReqType = X_QueryDeviceState;
 req-deviceid = dev-device_id;
 
-if (!_XReply(dpy, (xReply *)  rep, 0, xFalse)) {
-   UnlockDisplay(dpy);
-   SyncHandle();
-   return (XDeviceState *) NULL;
-}
+if (!_XReply(dpy, (xReply *)  rep, 0, xFalse))
+goto out;
 
 rlen = rep.length  2;
 if (rlen  0) {
data = Xmalloc(rlen);
if (!data) {
_XEatData(dpy, (unsigned long)rlen);
-   UnlockDisplay(dpy);
-   SyncHandle();
-   return ((XDeviceState *) NULL);
+   goto out;
}
_XRead(dpy, data, rlen);
 
@@ -117,11 +112,9 @@ XQueryDeviceState(
any = (XInputClass *) ((char *)any + any-length);
}
state = (XDeviceState *) Xmalloc(size + sizeof(XDeviceState));
-   if (!state) {
-   UnlockDisplay(dpy);
-   SyncHandle();
-   return ((XDeviceState *) NULL);
-   }
+   if (!state)
+goto out;
+
state-device_id = dev-device_id;
state-num_classes = rep.num_classes;
state-data = (XInputClass *) (state + 1);
@@ -175,8 +168,9 @@ XQueryDeviceState(
}
any = (XInputClass *) ((char *)any + any-length);
}
-   Xfree(data);
 }
+out:
+Xfree(data);
 
 UnlockDisplay(dpy);
 SyncHandle();
-- 
1.6.3.3

___
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: [Xorg-driver-geode] [PATCH 1/7] Prevent the pixmap migration if the geode GP can not do the acceleration.

2010-06-14 Thread Michel Dänzer

On Mon, 2010-06-14 at 01:50 +0300, Mart Raudsepp wrote: 
 On P, 2010-06-13 at 18:47 +0800, Huang, FrankR wrote:
 
  @@ -583,21 +586,23 @@ lx_check_composite(int op, PicturePtr pSrc, 
  PicturePtr pMsk, PicturePtr pDst)
  return FALSE;
   
   if (pMsk  op != PictOpClear) {
  +   struct blend_ops_t *opPtr = lx_alpha_ops[op * 2];
  +   int direction = (opPtr-channel == CIMGP_CHANNEL_A_SOURCE) ? 0 : 1;
  +
  +   /* Direction 0 indicates src-dst, 1 indiates dst-src */
  +   if (((direction == 0)  (pSrc-pDrawable-bitsPerPixel  16)) ||
  +   ((direction == 1)  (pDst-pDrawable-bitsPerPixel  16))) {
  +   ErrorF(Can't do mask blending with less then 16bpp\n);
  +   return FALSE;
  +   }
 snip 
  -   /* Direction 0 indicates src-dst, 1 indiates dst-src */
  -
  -   if (((direction == 0)  (pxSrc-drawable.bitsPerPixel  16)) ||
  -   ((direction == 1)  (pxDst-drawable.bitsPerPixel  16))) {
  -   ErrorF(Can't do mask blending with less then 16bpp\n);
  -   return FALSE;
  -   }
  -
 
 
 Are you sure this can be moved so easily? Is there any guarantee that if
 the src/dst Picture drawable is 16bpp, that then the src/dst pixmap
 drawable will be too? I'm not sure we know that absolutely surely, do
 we?

Yes, they have to match: pSrc/Dst-pDrawable is either pxSrc/Dst itself
or a window backed by it.


-- 
Earthling Michel Dänzer   |http://www.vmware.com
Libre software enthusiast |  Debian, X and DRI developer
___
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: Rendering in geode

2010-06-14 Thread Jonathan Morton

On Sun, 2010-06-13 at 16:47 +0800, Huang, FrankR wrote:
 You know when the driver does the composite, it is called in
 exaTryDriverComposite() function. You guess what? It give a driver a
 composite call that the source's width is less than srcX. That is
 impossible from my point.
 Make is more simple:
 A XRenderComposite() call below:
 XRenderComposite(dpy, PictOpOver,
 picture_10x10[0].pict,
 0, win-pict, 11, 0, 0, 0, 50, 50, 40, 40)
 
 The source is a 10x10 pixmap, but the srcX is 11. The
 rendering
 result is none! I think the driver should refuse that parameter to do
 the rendering. It will bring chaos operation. That is my thought.

Source (and mask) coordinates are allowed to go outside the image
involved - both positive and negative.  The effect of this depends on
the Repeat attribute on that image.

If Repeat is None (0), then samples outside the image return the
all-zeroes vector, even if the image doesn't have an alpha channel.
This is then sent through the rendering equation as normal.

If Repeat is Normal (1), then the coordinate is wrapped back into the
image using the modulus (%) operation.  This is your usual repeating
tile function.  As a special case, a 1x1 image with Repeat set to Normal
is the usual way of representing a solid colour.

Repeat values of Extend (2) and Mirror (3) are also possible.

Obviously, you should reject attribute values that you cannot support.
But it might happen that the hardware doesn't know how to do the None
mode properly by itself, and failing to support that at CheckComposite
time is pretty useless - but you don't know the coordinates until after
that point.

Let's suppose you support the Src and Over modes.  These need to be
handled differently if the coordinates can fall outside the source
image(s) in No-Repeat mode: Src turns to a fill of the all-zeroes
vector, and Over turns into a no-op.  You can therefore divide the
rendering area into normal and blanked zones, and issue separate
commands to the hardware for each.

Because Clear mode doesn't use the source or mask data at all, it is
unaffected by this quirk - but it is rarely used.  Other modes have
similar simplifications as Src and Over.

-- 
--
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

[PATCH] libxrandr: Check if getting screen for root fails

XRRRootToScreen might return -1 if it fails to find screen for the root
window. Following code uses screen number unconditionaly to index the screen
array.

Signed-off-by: Pauli Nieminen ext-pauli.niemi...@nokia.com
---
 src/Xrandr.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/Xrandr.c b/src/Xrandr.c
index 8ab1eae..3738d61 100644
--- a/src/Xrandr.c
+++ b/src/Xrandr.c
@@ -420,6 +420,9 @@ int XRRUpdateConfiguration(XEvent *event)
scevent = (XRRScreenChangeNotifyEvent *) event;
snum = XRRRootToScreen(dpy, 
   ((XRRScreenChangeNotifyEvent *) event)-root);
+   if (snum  0)
+   return 0;
+
if (scevent-rotation  (RR_Rotate_90 | RR_Rotate_270)) {
dpy-screens[snum].width   = scevent-height;
dpy-screens[snum].height  = scevent-width;
-- 
1.6.3.3

___
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 1/2] list.h: Fix list_for_each_entry_safe()

Can't use next as a macro argument since we're accessing the .next field
of struct list.
---
 include/list.h |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/list.h b/include/list.h
index 89dc29d..4ce20a8 100644
--- a/include/list.h
+++ b/include/list.h
@@ -94,10 +94,10 @@ list_is_empty(struct list *head)
 pos-member != (head);\
 pos = __container_of(pos-member.next, pos, member))
 
-#define list_for_each_entry_safe(pos, next, head, member)  \
+#define list_for_each_entry_safe(pos, tmp, head, member)   \
 for (pos = __container_of((head)-next, pos, member),  \
-next = __container_of(pos-member.next, pos, member);  \
+tmp = __container_of(pos-member.next, pos, member);   \
 pos-member != (head);\
-pos = next, next = __container_of(next-member.next, next, member))
+pos = tmp, tmp = __container_of(pos-member.next, tmp, member))
 
 #endif
-- 
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

[PATCH 2/2] xfixes: Use list.h macros in tracking hide count

---

This is not a patch to be applied.  Recently, the utility of list.h was
questioned and as I came a across the cursor hide count code today I
figured it would make a good example of how list.h can make the code
significantly simpler.  In particular, it's easy to prepend and iterate
though a singly linked list, but anything else is going to be buggy or slow
or best-case just 20 lines of code to delete an element that you shouldn't
have to write and debug again (cf deleteCursorHideCount() below).

Kristian

 xfixes/cursor.c |   47 +--
 1 files changed, 13 insertions(+), 34 deletions(-)

diff --git a/xfixes/cursor.c b/xfixes/cursor.c
index d3a207d..c8d03c7 100644
--- a/xfixes/cursor.c
+++ b/xfixes/cursor.c
@@ -53,6 +53,7 @@
 #include inputstr.h
 #include windowstr.h
 #include xace.h
+#include list.h
 
 static RESTYPE CursorClientType;
 static RESTYPE CursorHideCountType;
@@ -100,7 +101,7 @@ static CursorEventPtr   cursorEvents;
 typedef struct _CursorHideCountRec *CursorHideCountPtr;
 
 typedef struct _CursorHideCountRec {
-CursorHideCountPtr   pNext;  
+struct list  link;
 ClientPtrpClient;
 ScreenPtrpScreen;
 int  hideCount;
@@ -114,7 +115,8 @@ typedef struct _CursorHideCountRec {
 typedef struct _CursorScreen {
 DisplayCursorProcPtr   DisplayCursor;
 CloseScreenProcPtr CloseScreen;
-CursorHideCountPtr  pCursorHideCounts;
+struct list CursorHideCounts;
+
 } CursorScreenRec, *CursorScreenPtr;
 
 #define GetCursorScreen(s) 
((CursorScreenPtr)dixLookupPrivate((s)-devPrivates, CursorScreenPrivateKey))
@@ -146,7 +148,7 @@ CursorDisplayCursor (DeviceIntPtr pDev,
 if (ConnectionInfo)
CursorVisible = EnableCursor;
 
-if (cs-pCursorHideCounts != NULL || !CursorVisible) {
+if (!list_is_empty(cs-CursorHideCounts) || !CursorVisible) {
ret = (*pScreen-DisplayCursor) (pDev, pScreen, NullCursor);
 } else {
ret = (*pScreen-DisplayCursor) (pDev, pScreen, pCursor);
@@ -779,7 +781,7 @@ findCursorHideCount (ClientPtr pClient, ScreenPtr pScreen)
 CursorScreenPtrcs = GetCursorScreen(pScreen);
 CursorHideCountPtr pChc;
 
-for (pChc = cs-pCursorHideCounts; pChc != NULL; pChc = pChc-pNext) {
+list_for_each_entry(pChc, cs-CursorHideCounts, link) {
if (pChc-pClient == pClient) {
return pChc;
}
@@ -802,8 +804,7 @@ createCursorHideCount (ClientPtr pClient, ScreenPtr pScreen)
 pChc-pScreen = pScreen;
 pChc-hideCount = 1;
 pChc-resource = FakeClientID(pClient-index);
-pChc-pNext = cs-pCursorHideCounts;
-cs-pCursorHideCounts = pChc;
+list_add(pChc-link, cs-CursorHideCounts);
 
 /* 
  * Create a resource for this element so it can be deleted
@@ -822,27 +823,10 @@ createCursorHideCount (ClientPtr pClient, ScreenPtr 
pScreen)
  * Delete the given hide-counts list element from its screen list.
  */
 static void
-deleteCursorHideCount (CursorHideCountPtr pChcToDel, ScreenPtr pScreen)
+deleteCursorHideCount (CursorHideCountPtr pChcToDel)
 {
-CursorScreenPtrcs = GetCursorScreen(pScreen);
-CursorHideCountPtr pChc, pNext;
-CursorHideCountPtr pChcLast = NULL;
-
-pChc = cs-pCursorHideCounts;
-while (pChc != NULL) {
-   pNext = pChc-pNext;
-   if (pChc == pChcToDel) {
-   free(pChc);
-   if (pChcLast == NULL) {
-   cs-pCursorHideCounts = pNext;
-   } else {
-   pChcLast-pNext = pNext;
-   }
-   return;
-   }
-   pChcLast = pChc;
-   pChc = pNext;
-}
+list_del(pChcToDel-link);
+free(pChcToDel);
 }
 
 /* 
@@ -854,13 +838,8 @@ deleteCursorHideCountsForScreen (ScreenPtr pScreen)
 CursorScreenPtrcs = GetCursorScreen(pScreen);
 CursorHideCountPtr pChc, pTmp;
 
-pChc = cs-pCursorHideCounts;
-while (pChc != NULL) {
-   pTmp = pChc-pNext;
+list_for_each_entry_safe(pChc, pTmp, cs-CursorHideCounts, link)
FreeResource(pChc-resource, 0);
-   pChc = pTmp;
-}
-cs-pCursorHideCounts = NULL;   
 }
 
 int 
@@ -1002,7 +981,7 @@ CursorFreeHideCount (pointer data, XID id)
 ScreenPtr pScreen = pChc-pScreen;
 DeviceIntPtr dev;
 
-deleteCursorHideCount(pChc, pChc-pScreen);
+deleteCursorHideCount(pChc);
 for (dev = inputInfo.devices; dev; dev = dev-next)
 {
 if (IsMaster(dev)  IsPointerDevice(dev))
@@ -1047,7 +1026,7 @@ XFixesCursorInit (void)
return FALSE;
Wrap (cs, pScreen, CloseScreen, CursorCloseScreen);
Wrap (cs, pScreen, DisplayCursor, CursorDisplayCursor);
-   cs-pCursorHideCounts = NULL;
+   list_init(cs-CursorHideCounts);
SetCursorScreen (pScreen, cs);
 }
 CursorClientType = CreateNewResourceType(CursorFreeClient,
-- 
1.7.1

___
xorg-devel@lists.x.org: X.Org

Re: x.org documentation (extension tutorial)

2010-06-14 Thread Matt Dew

Tapani,
  Thanks for cleaning up my bad formatting on that wiki page. It looks
much better now. :)

Vincent,
Would a docbook generated page be of value?  It seems like having
it on the wiki is sufficient.I guess if one day we wanna have an
Xorg for Dummies book then that page would be good to have.

Matt

On Mon, Jun 14, 2010 at 2:13 PM, Tapani Pälli tapani.pa...@nokia.com wrote:
 On Sat, 2010-06-12 at 18:08 +0200, ext Alan Coopersmith wrote:
 Matt Dew wrote:
  Hi all,
     Tapani Palli over at maemo has a nice little extension tutorial over at:
  http://wiki.maemo.org/X11_Extension_tutorial
 
  It's moving to Xorg's wiki here:
  http://www.x.org/wiki/X11_Extension_tutorial

 Cool, thanks to both of you for working on this.

 I hope it is still useful as it might be a bit outdated. I have some
 more text which I intended to add to tutorial but never did, I will take
 a look if it still looks proper.

  I'm not knowledgable enough (yet. ;) ) to help update this page.
  Can people fill in the missing information?

 I've cc'd the xcb list as well, since going forward, we really should have
 adding xcb support as a best practice for new extensions, so hopefully 
 someone
 there can add a few words about that.

 Agreed, this would be very valuable.

 // Tapani



___
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] list.h: Fix list_for_each_entry_safe()

2010-06-14 Thread Tiago Vignatti

On Mon, Jun 14, 2010 at 03:25:22PM +0200, ext Kristian H�gsberg wrote:
 Can't use next as a macro argument since we're accessing the .next field
 of struct list.
 ---
  include/list.h |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/include/list.h b/include/list.h
 index 89dc29d..4ce20a8 100644
 --- a/include/list.h
 +++ b/include/list.h
 @@ -94,10 +94,10 @@ list_is_empty(struct list *head)
pos-member != (head);\
pos = __container_of(pos-member.next, pos, member))
  
 -#define list_for_each_entry_safe(pos, next, head, member)\
 +#define list_for_each_entry_safe(pos, tmp, head, member) \
  for (pos = __container_of((head)-next, pos, member),\
 -  next = __container_of(pos-member.next, pos, member);  \
 +  tmp = __container_of(pos-member.next, pos, member);   \
pos-member != (head);\
 -  pos = next, next = __container_of(next-member.next, next, member))
 +  pos = tmp, tmp = __container_of(pos-member.next, tmp, member))
  
  #endif

Kristian, doesn't make sense then to use inline function here instead keep
juggling with the names?

 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 2/2] xfixes: Use list.h macros in tracking hide count

2010-06-14 Thread Tiago Vignatti

On Mon, Jun 14, 2010 at 03:25:23PM +0200, ext Kristian H�gsberg wrote:
 
 This is not a patch to be applied.

why not?! :)

 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] Delete unused devPrivate field from GCFuncs and GCOps.

2010-06-14 Thread Tiago Vignatti

On Sun, Jun 13, 2010 at 01:20:40AM +0200, ext Jamey Sharp wrote:
 Signed-off-by: Jamey Sharp ja...@minilop.net
 ---
  hw/xfree86/common/xf86VGAarbiter.c |1 -
  hw/xfree86/shadowfb/shadow.c   |1 -
  hw/xfree86/xaa/xaaFallback.c   |1 -
  hw/xfree86/xaa/xaaGC.c |1 -
  include/gcstruct.h |3 ---
  miext/damage/damage.c  |1 -
  6 files changed, 0 insertions(+), 8 deletions(-)

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


FYI, we have also the privates implementation in Xorg DDX (privates field of
ScrnInfoRec). I don't see any particular reason to have another privates
mechanism inside Xorg given we have already the one in dix, so we might want
to get rid of it as well.


Thanks,

  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

Attach touch screen to a single display

2010-06-14 Thread Julian Ortiz

Hello,
After looking for a way to make a touch screen works only in its display
(with nvidia configured as different XScreens) with no luck at all, I
started to look at the input modules source. I realizes that
the proprietary elo module calls to xf86XInputSetScreen, the function name
seems promising, so I just patch the evdev module to support several screens
by configuration and to call xf86InputSetScreen in the EvdevReadInput just
before the EvdevProcessEvent.

The result seems to work but the behavior is very erratic and sometimes the
mouse/touch/keyboard get freezes and I need to restart X. And more strange,
after launch several OpenGL application the performance drops so, so much
(obviously without the patch it doesn't happen)

Is xf86InputSetScreen the best approach to handle this?

Trying to find the module responsible for the mouse switch between different
display I saw references to something called SilkenMouse, but I can't find
any documentation about what is this. Any idea?


Regards
Julian Ortiz
___
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] Delete xaaWrapper.

On Sun, Jun 13, 2010 at 11:54 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Sun, Jun 13, 2010 at 9:22 PM, Jamey Sharp ja...@minilop.net wrote:
 This was part of An experimental pseudocolor emulation layer. Not fully
 completed, currently only works for 16bpp. Only neomagic tried to use
 it, and that was neutered by the removal of the fbpseudocolor portion of
 that emulation layer; the rest is easily removed.

 Signed-off-by: Jamey Sharp ja...@minilop.net
 ---
 I sent the corresponding neomagic patch to the list a little while ago.

 IIRC, savage had something like this too using the overlay.

I have xf86-video-savage checked out and my grep over the drivers
didn't match any use of xaaWrapper outside neomagic. So I don't think
savage is an issue here?

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/2] list.h: Fix list_for_each_entry_safe()

On Mon, Jun 14, 2010 at 10:52 AM, Tiago Vignatti
tiago.vigna...@nokia.com wrote:
 On Mon, Jun 14, 2010 at 03:25:22PM +0200, ext Kristian H�gsberg wrote:
 Can't use next as a macro argument since we're accessing the .next field
 of struct list.
 ---
  include/list.h |    6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/include/list.h b/include/list.h
 index 89dc29d..4ce20a8 100644
 --- a/include/list.h
 +++ b/include/list.h
 @@ -94,10 +94,10 @@ list_is_empty(struct list *head)
        pos-member != (head);                                        \
        pos = __container_of(pos-member.next, pos, member))

 -#define list_for_each_entry_safe(pos, next, head, member)            \
 +#define list_for_each_entry_safe(pos, tmp, head, member)             \
      for (pos = __container_of((head)-next, pos, member),            \
 -      next = __container_of(pos-member.next, pos, member);          \
 +      tmp = __container_of(pos-member.next, pos, member);           \
        pos-member != (head);                                        \
 -      pos = next, next = __container_of(next-member.next, next, member))
 +      pos = tmp, tmp = __container_of(pos-member.next, tmp, member))

  #endif

 Kristian, doesn't make sense then to use inline function here instead keep
 juggling with the names?

We can't do that for this macro.  If you look closer, you'll see that
it expands to a for statement without the body.  You use it like this:

list_for_each_entry_safe(...) {
/* do something with each entry */
}

Kristian
___
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/2] xfixes: Use list.h macros in tracking hide count

On Mon, Jun 14, 2010 at 10:56 AM, Tiago Vignatti
tiago.vigna...@nokia.com wrote:
 On Mon, Jun 14, 2010 at 03:25:23PM +0200, ext Kristian H�gsberg wrote:

 This is not a patch to be applied.

 why not?! :)

Oh, I mentioned this in my first attempt to send out these patches
(which git send-mail threw away when it couldn't find an smtp server).
 I don't think it makes sense to go through the server and replace
existing linked list code.  That code is already written and tested,
and replacing it can only break stuff.  The patch was just meant to
illustrate how list.h easily pays for itself once you need to do
something more advanced than prepending or iterating through the list.
 You know, just in case Keith got tempted to rewrite more list.h users
to open coded lists.

Kristian
___
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] Delete xaaWrapper.

2010-06-14 Thread Alex Deucher

On Mon, Jun 14, 2010 at 11:11 AM, Jamey Sharp ja...@minilop.net wrote:
 On Sun, Jun 13, 2010 at 11:54 PM, Alex Deucher alexdeuc...@gmail.com wrote:
 On Sun, Jun 13, 2010 at 9:22 PM, Jamey Sharp ja...@minilop.net wrote:
 This was part of An experimental pseudocolor emulation layer. Not fully
 completed, currently only works for 16bpp. Only neomagic tried to use
 it, and that was neutered by the removal of the fbpseudocolor portion of
 that emulation layer; the rest is easily removed.

 Signed-off-by: Jamey Sharp ja...@minilop.net
 ---
 I sent the corresponding neomagic patch to the list a little while ago.

 IIRC, savage had something like this too using the overlay.

 I have xf86-video-savage checked out and my grep over the drivers
 didn't match any use of xaaWrapper outside neomagic. So I don't think
 savage is an issue here?

It's probably fine then.  It's been ages since I looked.

Alex
___
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/2] xfixes: Use list.h macros in tracking hide count

2010/6/14 Vignatti Tiago (Nokia-D/Helsinki) tiago.vigna...@nokia.com:
 On Mon, Jun 14, 2010 at 05:24:16PM +0200, ext Kristian Høgsberg wrote:

  I don't think it makes sense to go through the server and replace
 existing linked list code.  That code is already written and tested,
 and replacing it can only break stuff.

 I think open coded list implementation everywhere is more error prone than
 just have one single set of basic macros. Isn't it?

All the open coded lists in ther server are there now, and they're
working.  If it's not broken, don't mess with it.  If you come across
an error in an open coded list implementation, that would be a good
reason to port it to list.h.  But otherwise, you're just almost
certainly going to break working code.

Kristian
___
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] damage: Delete NOTUSED block--it was never not NOTUSED.

Signed-off-by: Jamey Sharp ja...@minilop.net
---
 miext/damage/damage.c |   22 --
 1 files changed, 0 insertions(+), 22 deletions(-)

diff --git a/miext/damage/damage.c b/miext/damage/damage.c
index 54431c9..f5917ea 100644
--- a/miext/damage/damage.c
+++ b/miext/damage/damage.c
@@ -462,28 +462,6 @@ damageCreateGC(GCPtr pGC)
 return ret;
 }
 
-#ifdef NOTUSED
-static void
-damageWrapGC (GCPtr pGC)
-{
-damageGCPriv(pGC);
-
-pGCPriv-ops = NULL;
-pGCPriv-funcs = pGC-funcs;
-pGC-funcs = damageGCFuncs;
-}
-
-static void
-damageUnwrapGC (GCPtr pGC)
-{
-damageGCPriv(pGC);
-
-pGC-funcs = pGCPriv-funcs;
-if (pGCPriv-ops)
-   pGC-ops = pGCPriv-ops;
-}
-#endif
-
 #define DAMAGE_GC_OP_PROLOGUE(pGC, pDrawable) \
 damageGCPriv(pGC);  \
 GCFuncs *oldFuncs = pGC-funcs; \
-- 
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: Who can explain the diff between Xserver-1.6.4 version and 1.7 version about the ExaGetPixmapAddress?

2010-06-14 Thread Cui, Hunk

Hi, Maarten  Michel,

Before 08/2008, our Geode-LX driver were use exaAllocOffscreen, but for 
update to Randr 1.2, Jordan Crouse replace exaAllocOffscreen with 
GeodeAllocOffscreen, now Jordan Crouse have been leave AMD, So I can not trace 
the change log.
About the change, you can see: 
http://cgit.freedesktop.org/xorg/driver/xf86-video-geode/commit/?id=d681a844e448712a9a419d2a4dca81930d39a80a
(It have been delete exaAllocOffscreen)

As you said Rotateddata has to be allocated between memoryBase and 
memorySize. Can you afford a structure of exaAllocOffscreen in another 
video-driver? How to allocate the memory in InitMemory? (e.g: ATI driver), then 
I can compare the diff.

I think the GeodeAllocOffscreen have been use for more than two years, 
It can always properly allocate memory. Now because the Xserver have been 
updated to 1.7 version, delete sys_ptr in exaGetPixmapOffset, therefore, cause 
this error.

Thanks,
Hunk Cui


-Original Message-
From: Michel Dänzer [mailto:mic...@daenzer.net] 
Sent: Monday, June 14, 2010 3:26 PM
To: Maarten Maathuis
Cc: Cui, Hunk; xorg-devel@lists.x.org; xorg-driver-ge...@lists.x.org
Subject: Re: Who can explain the diff between Xserver-1.6.4 version and 1.7 
version about the ExaGetPixmapAddress?

On Son, 2010-06-13 at 16:10 +0200, Maarten Maathuis wrote: 
 2010/6/13 Cui, Hunk hunk@amd.com:
  Hi, Maarten,
 
 In our xf86-video-geode driver, all of memories are allocated by 
  GeodeAllocOffscreen, the exa offscreen memory is part of the memorySize. 
  And the rotation data is not in memorySize, it is allocated after 
  memorySize.
 
 This is exactly the reason why exa doesn't recognize it. Rotateddata
 has to be allocated between memoryBase and memorySize. Only
 exaAllocOffscreen can do that.
 
 
 About the GeodeAllocOffscreen, it is Data structure tables, why you 
  said allocate exa offscreen memory out of a private pool? What the diff 
  between GeodeAllocOffscreen and exaOffscreen? In exaOffscreenInit, the EXA 
  offscreen base and size are loaded into server, so the exa should recognize 
  it as offscreen memory, furthermore, the ratate_memory(2MB) is not included 
  in EXA offscreen space.
 That pPixData - pExaScr-info-memoryBase = 
  pExaScr-info-memorySize should right.
 
 Just because it's right for your driver doesn't mean it's right
 everywhere (the memory after memorySize could belong to another device
 for example). Classic exa is made on the assumption that all memory
 usable for gpu acceleration is a linear range between memoryBase and
 memorySize. If you don't want this limitation then you should move to
 another type of exa where the driver has more control.
 
 I just don't see why you cannot use exaAllocOffscreen for rotatedData,
 that's what every driver has done and it works last i tried.
 
 What you have proposed so far is just a hack that will never be added.

Agreed. The geode driver should either allocate the memory with
exaOffscreenAlloc() or not rely on EXA facilities like
exaGetPixmapOffset() in the PixmapIsOffscreen driver hook.


-- 
Earthling Michel Dänzer   |http://www.vmware.com
Libre software enthusiast |  Debian, X and DRI developer

___
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] DRI2/xserver: Don't hang in glXSwapBuffers if drawable moves between crtc's

2010-06-14 Thread Jesse Barnes

On Sun, 13 Jun 2010 18:05:26 +0200
Mario Kleiner mario.klei...@tuebingen.mpg.de wrote:

 Detect if a drawable has been moved from an original crtc to a new crtc
 with a lower current vblank count than the original crtc inbetween
 glXSwapBuffers() calls. Reinitialize drawable's last_swap_target
 before scheduling next swap if such a move has taken place.
 
 last_swap_target defines the baseline for scheduling the next swap.
 If a movement between crtc's is not taken into account, the swap may
 schedule for a vblank count on the new crtc far in the future, resulting
 in a apparent hang of the drawable for a long time.
 
 Fixes Bugzilla bug #28383.
 
 Signed-off-by: Mario Kleiner mario.klei...@tuebingen.mpg.de
 ---
  hw/xfree86/dri2/dri2.c |   15 +++
  1 files changed, 15 insertions(+), 0 deletions(-)
 
 diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
 index d33b0d1..1f80022 100644
 --- a/hw/xfree86/dri2/dri2.c
 +++ b/hw/xfree86/dri2/dri2.c
 @@ -756,6 +756,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, 
 CARD64 target_msc,
  DRI2DrawablePtr pPriv;
  DRI2BufferPtr   pDestBuffer = NULL, pSrcBuffer = NULL;
  int ret, i;
 +CARD64  ust, current_msc;
  
  pPriv = DRI2GetDrawable(pDraw);
  if (pPriv == NULL) {
 @@ -800,12 +801,26 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, 
 CARD64 target_msc,
   * need to schedule a swap for the last swap target + the swap interval.
   */
  if (target_msc == 0  divisor == 0  remainder == 0) {
 + /* If the current vblank count of the drawable's crtc is lower
 +  * than the count stored in last_swap_target from a previous swap
 +  * then reinitialize last_swap_target to the current crtc's msc,
 +  * otherwise the swap will hang. This will happen if the drawable
 +  * is moved to a crtc with a lower refresh rate, or a crtc that just
 +  * got enabled.
 +  */
 + if (!(*ds-GetMSC)(pDraw, ust, current_msc))
 + pPriv-last_swap_target = 0;
 +
 + if (current_msc  pPriv-last_swap_target)
 + pPriv-last_swap_target = current_msc;
 +
   /*
* Swap target for this swap is last swap target + swap interval since
* we have to account for the current swap count, interval, and the
* number of pending swaps.
*/
   *swap_target = pPriv-last_swap_target + pPriv-swap_interval;
 +
  } else {
   /* glXSwapBuffersMscOML could have a 0 target_msc, honor it */
   *swap_target = target_msc;

Yeah, this looks ok.  Really we should write up GLX extension that
clarifies behavior in multi-head and display on/off situations too.  Is
that something you could do?

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
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: [PATCH 2/2] xfixes: Use list.h macros in tracking hide count

2010-06-14 Thread Vignatti Tiago (Nokia-D/Helsinki)

On Mon, Jun 14, 2010 at 05:48:54PM +0200, ext Kristian Høgsberg wrote:
 2010/6/14 Vignatti Tiago (Nokia-D/Helsinki) tiago.vigna...@nokia.com:
 
  I think open coded list implementation everywhere is more error prone than
  just have one single set of basic macros. Isn't it?
 
 All the open coded lists in ther server are there now, and they're
 working.  If it's not broken, don't mess with it.  If you come across
 an error in an open coded list implementation, that would be a good
 reason to port it to list.h.  But otherwise, you're just almost
 certainly going to break working code.
 

I disagree with you Kristian.

Pick any code related with clean-up we had done recently; for instance the
replacement of the memory allocation function wrappers by the C89 ones. We can
say that they were working, but this wouldn't be an argument to not remove.
If X would have a shiny code base and a driver API stable enough then I'd
agree with you to not touch. But you know we still have a way long to go to
achieve it. 

Seems that we're living in a era that Xorg project has more developers than
never [0], so the moment of clean up (including low-hanging fruits like this
list stuff) is right now.


Cheers,

[0] the traffic on xorg-devel on last months are _at_ _least_ 4 times higher
than one year ago.

 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 2/2] xfixes: Use list.h macros in tracking hide count

2010-06-14 Thread Mikhail Gusarov


Twas brillig at 19:16:46 14.06.2010 UTC+03 when tiago.vigna...@nokia.com did 
gyre and gimble:

  All the open coded lists in ther server are there now, and they're
  working.  If it's not broken, don't mess with it.  If you come
  across an error in an open coded list implementation, that would be
  a good reason to port it to list.h.  But otherwise, you're just
  almost certainly going to break working code.

 VT( I disagree with you Kristian.

 VT Pick any code related with clean-up we had done recently; for
 VT instance the replacement of the memory allocation function wrappers
 VT by the C89 ones. We can say that they were working, but this
 VT wouldn't be an argument to not remove.

Especially as this clean-up exposed several hidden bugs.

-- 
  http://fossarchy.blogspot.com/


pgpUyJjmLOh3G.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] xnest: Delete unused nClipRects GC-private field.

This field was never read at any time in the git history.

Signed-off-by: Jamey Sharp ja...@minilop.net
---
 hw/xnest/GC.c   |3 ---
 hw/xnest/XNGC.h |1 -
 2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/hw/xnest/GC.c b/hw/xnest/GC.c
index 2761583..4cfab29 100644
--- a/hw/xnest/GC.c
+++ b/hw/xnest/GC.c
@@ -84,7 +84,6 @@ xnestCreateGC(GCPtr pGC)
   xnestGCPriv(pGC)-gc = XCreateGC(xnestDisplay, 
   xnestDefaultDrawables[pGC-depth], 
   0L, NULL);
-  xnestGCPriv(pGC)-nClipRects = 0;
 
   return True;
 }
@@ -287,7 +286,6 @@ xnestChangeClip(GCPtr pGC, int type, pointer pValue, int 
nRects)
 
   pGC-clientClipType = type;
   pGC-clientClip = pValue;
-  xnestGCPriv(pGC)-nClipRects = nRects;
 }
 
 void
@@ -299,7 +297,6 @@ xnestDestroyClip(GCPtr pGC)
  
   pGC-clientClipType = CT_NONE;
   pGC-clientClip = NULL;
-  xnestGCPriv(pGC)-nClipRects = 0;
 }
 
 void
diff --git a/hw/xnest/XNGC.h b/hw/xnest/XNGC.h
index 9f10456..c4a6cef 100644
--- a/hw/xnest/XNGC.h
+++ b/hw/xnest/XNGC.h
@@ -19,7 +19,6 @@ is without express or implied warranty.
 
 typedef struct {
   XlibGC gc;
-  int nClipRects;
 } xnestPrivGC;
 
 extern DevPrivateKeyRec xnestGCPrivateKeyRec;
-- 
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

[PULL] cleanups

These cleanups and warning fixes have been reviewed and don't, as far as
I can tell, affect API or ABI. Can they be merged to master?

The following changes since commit 620ca54aaa0b363fcf68cec1bd6c37e68c988352:
  Keith Packard (1):
Merge remote branch 'alanc/master'

are available in the git repository at:

  git://people.freedesktop.org/~jamey/xserver for-keith

Jamey Sharp (5):
  dmx: Delete unused GLX visual matching code.
  dmx: __glXMalloc - malloc, etc.
  dmx: Delete '#undef Xmalloc' and friends.
  fb: Delete unused oneRect private field.
  damage: Delete NOTUSED block--it was never not NOTUSED.

 fb/fb.h   |1 -
 fb/fbgc.c |1 -
 hw/dmx/dmxinit.c  |9 -
 hw/dmx/glxProxy/glxcmds.c |   55 +++---
 hw/dmx/glxProxy/glxcmdsswap.c |4 +-
 hw/dmx/glxProxy/glxext.c  |   76 ++--
 hw/dmx/glxProxy/glxext.h  |   14 --
 hw/dmx/glxProxy/glxscreens.c  |5 -
 hw/dmx/glxProxy/glxsingle.c   |5 -
 hw/dmx/glxProxy/glxutil.c |   75 
 hw/dmx/glxProxy/glxutil.h |6 -
 hw/dmx/glxProxy/glxvendor.c   |5 -
 hw/dmx/glxProxy/glxvisuals.c  |  388 +
 hw/dmx/glxProxy/glxvisuals.h  |8 -
 miext/damage/damage.c |   22 ---
 15 files changed, 46 insertions(+), 628 deletions(-)


signature.asc
Description: Digital 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: Performance improvement to vga arbitration

2010-06-14 Thread Vignatti Tiago (Nokia-D/Helsinki)

Hi Henry,

On Sat, Jun 12, 2010 at 04:38:14AM +0200, ext Henry Zhao wrote:
On 06/11/10 04:56, Tiago Vignatti wrote:

http://people.freedesktop.org/~vignatti/tmp/0001-xfree86-vgaarb-disable-VGA-decoding-after-POST.patch

At this point on the X server, we already POSTed all cards and we could be
optimistic, letting the drivers say when we actually need VGA legacy. Right
now, as you said, we're assuming legacy access as default whenever the
system
has more than one card, not driven by DRI (DRI and VGA legacy conflict with
the current design. Dave preferred to let this away).

The patch didn't go to git master yet ?

not yet. Do you mind to do a proper review and insert your review tag there?

For the X, we have a strict and rigorous development process now, trying to
minimize the amount of errors being pushed upstream trees. For this we needed
to set up a way get patches reviewed by stamping a review tag. In general it's
not so different from what linux kernel is being doing. You may want to take a
look on these wiki pages:

http://www.x.org/wiki/XServer
http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches

We still need arbitration once a
device gets graphics mode. Say devices a and b are working in
graphics mode, then

(1) A new device c may start, which does legacy accesses during
initialization;
(2) While b is still running, device a may exit, which also involves
legacy accesses.

Therefore we need to use another lock (locks2), in addition to the
current locks
(for legacy accesses), for non-legacy accesses such that locks2 and
locks are in
conflict, but locks2s themselves are not.

Besides, we cannot rule out possibilities that some drivers may use legacy
access in some operations in graphics mode in certain cases, therefore
we need
to give drivers a final say on kind of accesses they use for the
operations, although
the default is non-legacy access.

right, I was missing the hotplug case. It makes perfect sense for me. And yes,
I'd prefer to keep VGA decoding disabled by default after the session is up,
exactly as my patch is doing.

Also, I posted two patches to fix the userspace decoding interface on the
mailing list some days ago. If you're carrying about remove cards from
arbitration and stop decoding then definitely you'll want to take a look on
those:

http://lists.x.org/archives/xorg-devel/2010-June/009559.html

I had this fixed in my patch.

can you please put your reviewing tags please?

3 patches are posted.

Your explanation was good enough and clear for me. However, in all of the three
patches, you're inserting more modifications than you described here. It makes
the review difficult and tough.

A good habit is to split one in a series in which each patch has a different
semantical meaning - one for cleaning up only, another introducing a hook
function, another for the actual changes and so on. If possible, if each patch
is independent from the other (able to compile) then it would also ease the
search for some possible errors (using git-bisect)

I'd please ask you to take a look on this page and re-send the patches, using
a proper git-format-patch style:

http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches

I have to admit that there are some parts of your kernel patch that simply
goes being the scope of my knowledge. I'm pretty sure if you follow this tips
on the wiki we're gonna be able to bring other guys from the community to do
the review.

Thank you,

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] Delete unused devPrivate field from GCFuncs and GCOps.

2010-06-14 Thread Keith Packard

On Mon, 14 Jun 2010 18:00:23 +0300, Tiago Vignatti tiago.vigna...@nokia.com 
wrote:

 FYI, we have also the privates implementation in Xorg DDX (privates field of
 ScrnInfoRec). I don't see any particular reason to have another privates
 mechanism inside Xorg given we have already the one in dix, so we might want
 to get rid of it as well.

We'd need to extend the semantics of the DIX implementation as the
ScrnInfoRec lifetime is server-long, not generation-long.

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


pgpKCP9Gh7zlv.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: Who can explain the diff between Xserver-1.6.4 version and 1.7 version about the ExaGetPixmapAddress?

2010-06-14 Thread Maarten Maathuis

2010/6/14 Cui, Hunk hunk@amd.com:
Hi, Maarten Michel,

Before 08/2008, our Geode-LX driver were use exaAllocOffscreen, but
for update to Randr 1.2, Jordan Crouse replace exaAllocOffscreen with
GeodeAllocOffscreen, now Jordan Crouse have been leave AMD, So I can not
trace the change log.
About the change, you can see:
http://cgit.freedesktop.org/xorg/driver/xf86-video-geode/commit/?id=d681a844e448712a9a419d2a4dca81930d39a80a
(It have been delete exaAllocOffscreen)

As you said Rotateddata has to be allocated between memoryBase and
memorySize. Can you afford a structure of exaAllocOffscreen in another
video-driver? How to allocate the memory in InitMemory? (e.g: ATI driver),
then I can compare the diff.

I think the GeodeAllocOffscreen have been use for more than two years,
It can always properly allocate memory. Now because the Xserver have been
updated to 1.7 version, delete sys_ptr in exaGetPixmapOffset, therefore,
cause this error.

You relied on behaviour that shouldn't have been defined in the first
place, we have given hints how to do it properly that will work in all
versions. One example of how exaAllocOffscreen is used can be found
here
http://cgit.freedesktop.org/xorg/driver/xf86-video-ati/tree/src/radeon_legacy_memory.c#n50.
area-offset is relative to memoryBase.

Thanks,
Hunk Cui

-Original Message-
From: Michel Dänzer [mailto:mic...@daenzer.net]
Sent: Monday, June 14, 2010 3:26 PM
To: Maarten Maathuis
Cc: Cui, Hunk; xorg-devel@lists.x.org; xorg-driver-ge...@lists.x.org
Subject: Re: Who can explain the diff between Xserver-1.6.4 version and 1.7
version about the ExaGetPixmapAddress?

On Son, 2010-06-13 at 16:10 +0200, Maarten Maathuis wrote:
2010/6/13 Cui, Hunk hunk@amd.com:
Hi, Maarten,

In our xf86-video-geode driver, all of memories are allocated by
GeodeAllocOffscreen, the exa offscreen memory is part of the memorySize.
And the rotation data is not in memorySize, it is allocated after
memorySize.

This is exactly the reason why exa doesn't recognize it. Rotateddata
has to be allocated between memoryBase and memorySize. Only
exaAllocOffscreen can do that.

About the GeodeAllocOffscreen, it is Data structure tables, why you
said allocate exa offscreen memory out of a private pool? What the diff
between GeodeAllocOffscreen and exaOffscreen? In exaOffscreenInit, the EXA
offscreen base and size are loaded into server, so the exa should
recognize it as offscreen memory, furthermore, the ratate_memory(2MB) is
not included in EXA offscreen space.
That pPixData - pExaScr-info-memoryBase =
pExaScr-info-memorySize should right.

Just because it's right for your driver doesn't mean it's right
everywhere (the memory after memorySize could belong to another device
for example). Classic exa is made on the assumption that all memory
usable for gpu acceleration is a linear range between memoryBase and
memorySize. If you don't want this limitation then you should move to
another type of exa where the driver has more control.

I just don't see why you cannot use exaAllocOffscreen for rotatedData,
that's what every driver has done and it works last i tried.

What you have proposed so far is just a hack that will never be added.

Agreed. The geode driver should either allocate the memory with
exaOffscreenAlloc() or not rely on EXA facilities like
exaGetPixmapOffset() in the PixmapIsOffscreen driver hook.

--
Earthling Michel Dänzer | http://www.vmware.com
Libre software enthusiast | Debian, X and DRI developer

--
Life spent, a precious moment, in the wink of an eye we live and we die.
___
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] Delete redundant GC initializations.

When a GC is allocated, it is zeroed, including all storage requested
with dixRegisterPrivateKey. So CreateGC hooks don't need to initialize
anything to zero.

Signed-off-by: Jamey Sharp ja...@minilop.net
---
This subsumes my earlier Delete redundant clip initializations patch.

 fb/fbgc.c |3 ---
 hw/xnest/GC.c |3 ---
 hw/xwin/wingc.c   |5 -
 miext/cw/cw.c |1 -
 miext/damage/damage.c |1 -
 5 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/fb/fbgc.c b/fb/fbgc.c
index 2568698..7a73e0d 100644
--- a/fb/fbgc.c
+++ b/fb/fbgc.c
@@ -64,9 +64,6 @@ const GCOps   fbGCOps = {
 Bool
 fbCreateGC(GCPtr pGC)
 {
-pGC-clientClip = NULL;
-pGC-clientClipType = CT_NONE;
-
 pGC-ops = (GCOps *) fbGCOps;
 pGC-funcs = (GCFuncs *) fbGCFuncs;
 
diff --git a/hw/xnest/GC.c b/hw/xnest/GC.c
index 4cfab29..e325d04 100644
--- a/hw/xnest/GC.c
+++ b/hw/xnest/GC.c
@@ -73,9 +73,6 @@ static GCOps xnestOps = {
 Bool
 xnestCreateGC(GCPtr pGC)
 {
-  pGC-clientClipType = CT_NONE;
-  pGC-clientClip = NULL;
-  
   pGC-funcs = xnestFuncs;
   pGC-ops = xnestOps;
   
diff --git a/hw/xwin/wingc.c b/hw/xwin/wingc.c
index 196b5b5..e351c50 100644
--- a/hw/xwin/wingc.c
+++ b/hw/xwin/wingc.c
@@ -137,11 +137,6 @@ winCreateGCNativeGDI (GCPtr pGC)
  pGC-depth);
 #endif
 
-  pGC-clientClip = NULL;
-  pGC-clientClipType = CT_NONE;
-  pGC-freeCompClip = FALSE;
-  pGC-pCompositeClip = 0;
-
   pGC-ops = (GCOps *) winGCOps;
   pGC-funcs = (GCFuncs *) winGCFuncs;
 
diff --git a/miext/cw/cw.c b/miext/cw/cw.c
index 58816c9..3da3bc3 100644
--- a/miext/cw/cw.c
+++ b/miext/cw/cw.c
@@ -325,7 +325,6 @@ cwCreateGC(GCPtr pGC)
 ScreenPtr  pScreen = pGC-pScreen;
 Bool   ret;
 
-memset(pPriv, 0, sizeof(cwGCRec));
 SCREEN_PROLOGUE(pScreen, CreateGC);
 
 if ( (ret = (*pScreen-CreateGC)(pGC)) )
diff --git a/miext/damage/damage.c b/miext/damage/damage.c
index 2318a61..eaaf0ef 100644
--- a/miext/damage/damage.c
+++ b/miext/damage/damage.c
@@ -450,7 +450,6 @@ damageCreateGC(GCPtr pGC)
 damageGCPriv(pGC);
 Bool ret;
 
-pGC-pCompositeClip = 0;
 unwrap (pScrPriv, pScreen, CreateGC);
 if((ret = (*pScreen-CreateGC) (pGC))) {
pGCPriv-ops = NULL;
-- 
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

[PATCH] Delete redundant GC initializations.