Re: [PATCH] dri2: Pass a ScreenPtr through to the driver's AuthMagic function.

2012-06-19 Thread Kristian Høgsberg
On Mon, Jun 18, 2012 at 12:58 PM, Keith Packard kei...@keithp.com wrote:
 Christopher James Halse Rogers christopher.halse.rog...@canonical.com
 writes:

 +typedef int (*DRI2AuthMagic2ProcPtr) (ScreenPtr pScreen, int fd, uint32_t 
 magic);

 Bikeshed -- seems like the 'fd' parameter is not needed in this API?
 I'll note that in the implementation of the wrapper, you pull it from
 the screen private instead of using the provided parameter...

Oh, that is a good point, I like that bikeshed color better.

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] Add APIs for selecting on write fds.

2012-06-19 Thread Keith Packard
This adds API support for waking the server when an fd becomes
writable.

void AddWriteSocket(int fd);

Add an FD to the select write mask; the server will wake when
this fd is writable.

void RemoveWriteSocket(int fd);

Remove the FD from the select write mask.

Note that there is no automated mechanism for emptying the set of
fds. The caller is responsible for removing the fd whenever
appropriate, including when the fd is closed, and when the
user no longer has pending write data.

Bool CheckWriteSocket(int fd);

Checks if the last select call marked this fd as
writable. This value persists until select is called again.

This patch also adds a helper function for sockets used for reading:

Bool CheckReadSocket(int fd);

A matching function that checks for fds marked as readable
that have been added with AddGeneralSocket or
AddEnabledDevice.

The mechanism used is to keep a separate fd_set of write sockets, and
OR that with the client write mask to create a global select write
mask. The only other change necessary in WaitFor is to mask out the
resulting non-client fds when checking for unblocked client sockets.

Signed-off-by: Keith Packard kei...@keithp.com
---
 include/os.h |8 ++
 os/WaitFor.c |   79 +++---
 2 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/include/os.h b/include/os.h
index 276eb52..efb2b65 100644
--- a/include/os.h
+++ b/include/os.h
@@ -157,6 +157,14 @@ extern _X_EXPORT void MakeClientGrabImpervious(ClientPtr 
/*client */ );
 
 extern _X_EXPORT void MakeClientGrabPervious(ClientPtr /*client */ );
 
+extern _X_EXPORT void AddWriteSocket(int fd);
+
+extern _X_EXPORT void RemoveWriteSocket(int fd);
+
+extern _X_EXPORT Bool CheckWriteSocket(int fd);
+
+extern _X_EXPORT Bool CheckReadSocket(int fd);
+
 #ifdef XQUARTZ
 extern void ListenOnOpenFD(int /* fd */ , int /* noxauth */ );
 #endif
diff --git a/os/WaitFor.c b/os/WaitFor.c
index 393890f..bf03273 100644
--- a/os/WaitFor.c
+++ b/os/WaitFor.c
@@ -123,6 +123,62 @@ struct _OsTimerRec {
 static void DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev);
 static void CheckAllTimers(void);
 static OsTimerPtr timers = NULL;
+static fd_set WriteSelectMask;
+static Bool AnyWriteSelectMask;
+static fd_set LastSelectWriteMask;
+
+/*
+ * AddWriteSocket:
+ * Add 'fd' to the list of write descriptors that will
+ * wake the server from select.
+ */
+
+void
+AddWriteSocket(int fd)
+{
+FD_SET(fd, WriteSelectMask);
+AnyWriteSelectMask = TRUE;
+}
+
+/*
+ * RemoveWriteSocket:
+ * Remove 'fd' from the list of write descriptors that will
+ * wake the server from select.
+ */
+
+void
+RemoveWriteSocket(int fd)
+{
+FD_CLR(fd, WriteSelectMask);
+AnyWriteSelectMask = XFD_ANYSET(WriteSelectMask);
+}
+
+/*
+ * CheckWriteSocket:
+ * Called from a WakeupHandler to check of 'fd'
+ * was marked as writable during the last select
+ * call
+ */
+
+Bool
+CheckWriteSocket(int fd)
+{
+return AnyWriteSelectMask  FD_ISSET(fd, LastSelectWriteMask);
+}
+
+/*
+ * CheckReadSocket:
+ * Called from a WakeupHandler to check of 'fd'
+ * was marked as readable during the last select
+ * call
+ */
+
+Bool
+CheckReadSocket(int fd)
+{
+return FD_ISSET(fd, LastSelectMask);
+}
+
 
 /*
  * WaitForSomething:
@@ -213,9 +269,11 @@ WaitForSomething(int *pClientsReady)
 /* keep this check close to select() call to minimize race */
 if (dispatchException)
 i = -1;
-else if (AnyClientsWriteBlocked) {
-XFD_COPYSET(ClientsWriteBlocked, clientsWritable);
-i = Select(MaxClients, LastSelectMask, clientsWritable, NULL, 
wt);
+else if (AnyClientsWriteBlocked || AnyWriteSelectMask) {
+XFD_COPYSET(ClientsWriteBlocked, LastSelectWriteMask);
+   if (AnyWriteSelectMask)
+   XFD_ORSET(LastSelectWriteMask, WriteSelectMask, 
LastSelectWriteMask);
+i = Select(MaxClients, LastSelectMask, LastSelectWriteMask, 
NULL, wt);
 }
 else {
 i = Select(MaxClients, LastSelectMask, NULL, NULL, wt);
@@ -285,12 +343,15 @@ WaitForSomething(int *pClientsReady)
 }
 if (someReady)
 XFD_ORSET(LastSelectMask, ClientsWithInput, LastSelectMask);
-if (AnyClientsWriteBlocked  XFD_ANYSET(clientsWritable)) {
-NewOutputPending = TRUE;
-XFD_ORSET(OutputPending, clientsWritable, OutputPending);
-XFD_UNSET(ClientsWriteBlocked, clientsWritable);
-if (!XFD_ANYSET(ClientsWriteBlocked))
-AnyClientsWriteBlocked = FALSE;
+if (AnyClientsWriteBlocked) {
+   XFD_ANDSET(clientsWritable, LastSelectWriteMask, 
ClientsWriteBlocked);
+ 

[PATCH] Test WaitForSomething select return in new Check* functions

2012-06-19 Thread Keith Packard
The bitmask from select is only valid if it returned a positive value,
so record it for use by CheckWriteSocket and CheckReadSocket

Signed-off-by: Keith Packard kei...@keithp.com
---
 os/WaitFor.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/os/WaitFor.c b/os/WaitFor.c
index bf03273..8f6f6eb 100644
--- a/os/WaitFor.c
+++ b/os/WaitFor.c
@@ -126,6 +126,7 @@ static OsTimerPtr timers = NULL;
 static fd_set WriteSelectMask;
 static Bool AnyWriteSelectMask;
 static fd_set LastSelectWriteMask;
+static int LastSelectReturn;
 
 /*
  * AddWriteSocket:
@@ -163,7 +164,7 @@ RemoveWriteSocket(int fd)
 Bool
 CheckWriteSocket(int fd)
 {
-return AnyWriteSelectMask  FD_ISSET(fd, LastSelectWriteMask);
+return LastSelectReturn  0  AnyWriteSelectMask  FD_ISSET(fd, 
LastSelectWriteMask);
 }
 
 /*
@@ -176,7 +177,7 @@ CheckWriteSocket(int fd)
 Bool
 CheckReadSocket(int fd)
 {
-return FD_ISSET(fd, LastSelectMask);
+return LastSelectReturn  0  FD_ISSET(fd, LastSelectMask);
 }
 
 
@@ -279,6 +280,7 @@ WaitForSomething(int *pClientsReady)
 i = Select(MaxClients, LastSelectMask, NULL, NULL, wt);
 }
 selecterr = GetErrno();
+   LastSelectReturn = i;
 WakeupHandler(i, (pointer) LastSelectMask);
 if (i = 0) {   /* An error or timeout occurred */
 if (dispatchException)
-- 
1.7.10

___
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: Pass a ScreenPtr through to the driver's AuthMagic function.

2012-06-19 Thread Christopher James Halse Rogers
On Tue, 2012-06-19 at 14:38 -0400, Kristian Høgsberg wrote:
 On Mon, Jun 18, 2012 at 12:58 PM, Keith Packard kei...@keithp.com wrote:
  Christopher James Halse Rogers christopher.halse.rog...@canonical.com
  writes:
 
  +typedef int (*DRI2AuthMagic2ProcPtr) (ScreenPtr pScreen, int fd, uint32_t 
  magic);
 
  Bikeshed -- seems like the 'fd' parameter is not needed in this API?
  I'll note that in the implementation of the wrapper, you pull it from
  the screen private instead of using the provided parameter...
 
 Oh, that is a good point, I like that bikeshed color better.
 

Technically I pull it from the DRI2Srceen private, which the driver
won't have access to, but {intel,nouveau,ati} also store this fd in
their driver private, so it's not a problem for them.

I'll send a blue-painted patch. :)


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

[PATCH] dri2: Pass a ScreenPtr through to the driver's AuthMagic function. (v3)

2012-06-19 Thread Christopher James Halse Rogers
xwayland drivers need access to their screen private data to authenticate.
Now that drivers no longer have direct access to the global screen arrays,
this needs to be passed in as function context.

v2: Don't break ABI
v3: Paint the bikeshed blue; drop fd from AuthMagic2ProcPtr prototype

Signed-off-by: Christopher James Halse Rogers 
christopher.halse.rog...@canonical.com
---
 hw/xfree86/dri2/dri2.c |   35 ---
 hw/xfree86/dri2/dri2.h |9 -
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index babf32f..d0f1789 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -104,7 +104,8 @@ typedef struct _DRI2Screen {
 DRI2ScheduleSwapProcPtr ScheduleSwap;
 DRI2GetMSCProcPtr GetMSC;
 DRI2ScheduleWaitMSCProcPtr ScheduleWaitMSC;
-DRI2AuthMagicProcPtr AuthMagic;
+DRI2AuthMagic2ProcPtr AuthMagic;
+DRI2AuthMagicProcPtr LegacyAuthMagic;
 DRI2ReuseBufferNotifyProcPtr ReuseBufferNotify;
 DRI2SwapLimitValidateProcPtr SwapLimitValidate;
 DRI2GetParamProcPtr GetParam;
@@ -1110,12 +,22 @@ DRI2Connect(ScreenPtr pScreen, unsigned int driverType, 
int *fd,
 return TRUE;
 }
 
+static Bool
+DRI2AuthMagic (ScreenPtr pScreen, uint32_t magic)
+{
+DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
+if (ds == NULL || (*ds-LegacyAuthMagic) (ds-fd, magic))
+return FALSE;
+
+return TRUE;
+}
+
 Bool
 DRI2Authenticate(ScreenPtr pScreen, uint32_t magic)
 {
 DRI2ScreenPtr ds = DRI2GetScreen(pScreen);
 
-if (ds == NULL || (*ds-AuthMagic) (ds-fd, magic))
+if (ds == NULL || (*ds-AuthMagic) (pScreen, magic))
 return FALSE;
 
 return TRUE;
@@ -1202,8 +1213,11 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
 cur_minor = 1;
 }
 
+if (info-version = 8) {
+ds-AuthMagic = info-AuthMagic2;
+}
 if (info-version = 5) {
-ds-AuthMagic = info-AuthMagic;
+ds-LegacyAuthMagic = info-AuthMagic;
 }
 
 if (info-version = 6) {
@@ -1218,14 +1232,21 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
 
 /*
  * if the driver doesn't provide an AuthMagic function or the info struct
- * version is too low, it relies on the old method (using libdrm) or fail
+ * version is too low, call through LegacyAuthMagic
  */
-if (!ds-AuthMagic)
+if (!ds-AuthMagic) {
+ds-AuthMagic = DRI2AuthMagic;
+/*
+ * If the driver doesn't provide an AuthMagic function
+ * it relies on the old method (using libdrm) or fails
+ */
+if (!ds-LegacyAuthMagic)
 #ifdef WITH_LIBDRM
-ds-AuthMagic = drmAuthMagic;
+ds-LegacyAuthMagic = drmAuthMagic;
 #else
-goto err_out;
+goto err_out;
 #endif
+}
 
 /* Initialize minor if needed and set to minimum provied by DDX */
 if (!dri2_minor || dri2_minor  cur_minor)
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index f849be6..4fd0fbc 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -64,6 +64,7 @@ typedef void (*DRI2CopyRegionProcPtr) (DrawablePtr pDraw,
DRI2BufferPtr pSrcBuffer);
 typedef void (*DRI2WaitProcPtr) (WindowPtr pWin, unsigned int sequence);
 typedef int (*DRI2AuthMagicProcPtr) (int fd, uint32_t magic);
+typedef int (*DRI2AuthMagic2ProcPtr) (ScreenPtr pScreen, uint32_t magic);
 
 /**
  * Schedule a buffer swap
@@ -192,7 +193,7 @@ typedef int (*DRI2GetParamProcPtr) (ClientPtr client,
 /**
  * Version of the DRI2InfoRec structure defined in this header
  */
-#define DRI2INFOREC_VERSION 7
+#define DRI2INFOREC_VERSION 8
 
 typedef struct {
 unsigned int version;   /** Version of this struct */
@@ -229,6 +230,12 @@ typedef struct {
 /* added in version 7 */
 
 DRI2GetParamProcPtr GetParam;
+
+/* added in version 8 */
+/* AuthMagic callback which passes extra context */
+/* If this is NULL the AuthMagic callback is used */
+/* If this is non-NULL the AuthMagic callback is ignored */
+DRI2AuthMagic2ProcPtr AuthMagic2;
 } DRI2InfoRec, *DRI2InfoPtr;
 
 extern _X_EXPORT int DRI2EventBase;
-- 
1.7.10.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: [PATCH] Add APIs for selecting on write fds.

2012-06-19 Thread Keith Packard
Peter Hutterer peter.hutte...@who-t.net writes:

 On Tue, Jun 19, 2012 at 12:00:12PM -0700, Keith Packard wrote:

 shouldn't the documentatin for the calls be in the code? looking up git
 commits to understand functions doesn't scale too well.

Yeah, I started by copying the text from the function definitions. I'll
clean those up.


 please fix your indentation settings, these should be spaces, not tabs.
 Acked-by: Peter Hutterer peter.hutte...@who-t.net otherwise

Heh. Yeah, I clearly haven't been *writing* a lot of X server code of
late :-)

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


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