Re: [PATCH] drm: Fix lock order reversal between mmap_sem and struct_mutex.

2009-02-20 Thread Thomas Hellstrom
Peter Zijlstra wrote:
 On Thu, 2009-02-19 at 22:02 +0100, Thomas Hellstrom wrote:
   
  
 It looks to me like the driver preferred locking order is

 object_mutex (which happens to be the device global struct_mutex)
   mmap_sem
  offset_mutex.

 So if one could avoid using the struct_mutex for object bookkeeping (A 
 separate lock) then
 vm_open() and vm_close() would adhere to that locking order as well, 
 simply by not taking the struct_mutex at all.

 So only fault() remains, in which that locking order is reversed. 
 Personally I think the trylock -reschedule-retry method with proper 
 commenting is a good solution. It will be the _only_ place where locking 
 order is reversed and it is done in a deadlock-safe manner. Note that 
 fault() doesn't really fail, but requests a retry from user-space with 
 rescheduling to give the process holding the struct_mutex time to 
 release it.
 

 It doesn't do the reschedule -- need_resched() will check if the current
 task was marked to be scheduled away, 
Yes. my mistake. set_tsk_need_resched() would be the proper call. If I'm 
correctly informed, that would kick in the scheduler _after_ the 
mmap_sem() is released, just before returning to user-space.
 furthermore yield based locking
 sucks chunks.
   
Yes, but AFAICT in this situation it is the only way to reverse locking 
order in a deadlock safe manner. If there is a lot of contention it will 
eat cpu. Unfortunately since the struct_mutex is such a wide lock there 
will probably be contention in some situations.

BTW isn't this quite common in distributed resource management, when you 
can't ensure that all requestors will request resources in the same order?
Try to grab all resources you need for an operation. If you fail to get 
one, release the resources you already have, sleep waiting for the 
failing one to be available and then retry.

In this case we fail be cause we _may_ have a deadlock. Since we cannot 
release the mmap_sem and wait, we do the second best thing and tell the 
kernel to reschedule when the mmap_sem is released.

 What's so very difficult about pulling the copy_*_user() out from under
 the locks?

   
Given Eric's comment this is a GEM performance-critical path, so even 
from a CPU-usage perspecive, the trylock solution may be preferred.

 From a make-the-code-easy-to-understand perspective, I agree pulling 
out copy_*_user() is a better solution.  It might even be that the 
trylock solution doesn't kill the warnings from the lock dependency tracker.

/Thomas



--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [PATCH] drm: Only use DRM_IOCTL_UPDATE_DRAW compat wrapper for compat X86.

2009-02-20 Thread David Miller
From: Arnd Bergmann a...@arndb.de
Date: Thu, 19 Feb 2009 15:19:01 +0100

 On Wednesday 18 February 2009, David Miller wrote:
  drm: Only use DRM_IOCTL_UPDATE_DRAW compat wrapper for compat X86.
  
  Only X86 32-bit uses a different alignment for unsigned long long
  than it's 64-bit counterpart.
  
  Therefore this compat translation is only correct, and only needed,
  when either CONFIG_X86 or CONFIG_IA64.
  
  Signed-off-by: David S. Miller da...@davemloft.net
 
 The patch is correct AFAICT, but I'd like to point out that the
 problem could have been avoided (besides using a non-padded layout)
 by using a compat_u64 member in the struct definition instead of
 the packed attribute:

Indeed, David A. showed me compat_u64 et al. and I'm fine with
it being fixed that way too.  Feel free to submit a patch :)

--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [PATCH] drm: Preserve SHMLBA bits in hash key for _DRM_SHM mappings.

2009-02-20 Thread David Miller
From: Andrew Morton a...@linux-foundation.org
Date: Thu, 19 Feb 2009 15:27:26 -0800

 eg:
 
 arch/xtensa/include/asm/shmparam.h
 #define SHMLBA  ((PAGE_SIZE  DCACHE_WAY_SIZE)? PAGE_SIZE : DCACHE_WAY_SIZE)
 
 
 But including linux/shm.h here seems a bit silly.   We'll see..

If DRM even builds on XTENSA, let alone is usable there, I'll
buy you a lollipop.

--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [PATCH] drm: Fix lock order reversal between mmap_sem and struct_mutex.

2009-02-20 Thread Peter Zijlstra
On Fri, 2009-02-20 at 09:31 +0100, Thomas Hellstrom wrote:
 Peter Zijlstra wrote:
  On Thu, 2009-02-19 at 22:02 +0100, Thomas Hellstrom wrote:

   
  It looks to me like the driver preferred locking order is
 
  object_mutex (which happens to be the device global struct_mutex)
mmap_sem
   offset_mutex.
 
  So if one could avoid using the struct_mutex for object bookkeeping (A 
  separate lock) then
  vm_open() and vm_close() would adhere to that locking order as well, 
  simply by not taking the struct_mutex at all.
 
  So only fault() remains, in which that locking order is reversed. 
  Personally I think the trylock -reschedule-retry method with proper 
  commenting is a good solution. It will be the _only_ place where locking 
  order is reversed and it is done in a deadlock-safe manner. Note that 
  fault() doesn't really fail, but requests a retry from user-space with 
  rescheduling to give the process holding the struct_mutex time to 
  release it.
  
 
  It doesn't do the reschedule -- need_resched() will check if the current
  task was marked to be scheduled away, 

 Yes. my mistake. set_tsk_need_resched() would be the proper call. If I'm 
 correctly informed, that would kick in the scheduler _after_ the 
 mmap_sem() is released, just before returning to user-space.

Yes, but it would still life-lock in the RT example given in the other
email.

  furthermore yield based locking
  sucks chunks.

 Yes, but AFAICT in this situation it is the only way to reverse locking 
 order in a deadlock safe manner. If there is a lot of contention it will 
 eat cpu. Unfortunately since the struct_mutex is such a wide lock there 
 will probably be contention in some situations.

I'd be surprised if this were the only solution. Maybe its the easiest,
but not one I'll support.

 BTW isn't this quite common in distributed resource management, when you 
 can't ensure that all requestors will request resources in the same order?
 Try to grab all resources you need for an operation. If you fail to get 
 one, release the resources you already have, sleep waiting for the 
 failing one to be available and then retry.

Not if you're building deterministic systems. Such constructs are highly
non-deterministic.

Furthermore, this isn't really a distributed system is it?


--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [PATCH] drm: Preserve SHMLBA bits in hash key for _DRM_SHM mappings.

2009-02-20 Thread Andrew Morton
On Fri, 20 Feb 2009 00:54:14 -0800 (PST) David Miller da...@davemloft.net 
wrote:

 From: Andrew Morton a...@linux-foundation.org
 Date: Thu, 19 Feb 2009 15:27:26 -0800
 
  eg:
  
  arch/xtensa/include/asm/shmparam.h
  #define SHMLBA  ((PAGE_SIZE  DCACHE_WAY_SIZE)? PAGE_SIZE : DCACHE_WAY_SIZE)
  
  
  But including linux/shm.h here seems a bit silly.   We'll see..
 
 If DRM even builds on XTENSA, let alone is usable there, I'll
 buy you a lollipop.

heh, OK.  But the risk is there.

tries to test sparc32 allmodconfig

scripts/mod/empty.c:1: error: -m64 is not supported by this configuration
scripts/mod/empty.c:1: error: -mlong-double-64 not allowed with -m64
scripts/mod/empty.c:1: error: -mcmodel= is not supported on 32 bit systems

retires hurt


--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [PATCH] i915: add page flipping ioctl

2009-02-20 Thread Chris Wilson
On Thu, 2009-02-19 at 16:43 -0800, Jesse Barnes wrote:
 On Thursday 19 February 2009 11:37:01 Chris Wilson wrote:
  With a few additional suggestions by Jesse, I've managed to get
  tear-free compositing working on i915. Here's the diff on top of the
  original patch (though obviously this is just a suggestion, still need
  to prevent multiple pending flips to the same plane and ensure that the
  old buffer is eventually unpinned, and you might choose to drop the
  mutex around the wait_for_vblank ;-):
 
 Yeah, looks pretty reasonable.  Here's what I've been working with.  It adds 
 a 
 couple of more changes (and slightly different cleanups) over your version:
   - added a seqno to wait for
   - wait for flip before queuing another
 but it's missing thew 915 changes you added (btw did I get the pitch wrong?  
 or are you submitting a different value for your objects?).

No, you wrote pitch = (stride / 8) * 8, I was halfway through
converting that into an -EINVAL guard as opposed to rounding down an
invalid stride.

 I added a new seqno since it's possible (even likely) that the next vblank 
 won't actually reflect the flip, if the GPU is busy processing a large 
 batchbuffer when the ring command goes in for example.

Yes, that might explain the rare tearing glitch I see.

 Also it might be a good idea to wait for any previous flip before queuing a 
 new one.  Anyway still tracking down some issues with the X side, but it 
 seems 
 like it's approaching readiness.

Give or take not checking for errors and not holding the mutex for long
enough. ;-)
-ickle


--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [Intel-gfx] [PATCH] i915: add page flipping ioctl

2009-02-20 Thread Chris Wilson
On Fri, 2009-02-20 at 13:46 +0800, Zou, Nanhai wrote:
 +struct drm_i915_gem_page_flip {
 +/** Handle of new front buffer */
 
  Should this be handle of new front buffer or handle of the execbuf?

I can't see how this can be an execbuf here. Do you mind elaborating?

Anyway this reminded me that we want a buffer offset along with the
handle.

 +uint32_t handle;
 +
 +/**
 + * page flip flags (wait on flip only for now)
 + */
 +uint32_t flags;
 +
 +/**
 + * pipe to flip
 + */
 +uint32_t pipe;
 +
 +/**
 + * screen dimensions for flip
 + */
 +uint32_t x;
 +uint32_t y;
 +};
 +
  #endif  /* _I915_DRM_H_ */



--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Acid-like effect in dark regions on key press

2009-02-20 Thread Chris Wilson
Hello all,

I'm seeing a bizarre problem whilst running wayland/cairo-drm/i915 under
KMS. Occasionally after pressing a key (which is handled by wayland
through the input layer) the dark regions (I'm estimating where the
value is less than ~4) become garbage. I've managed to capture a
screenshot before and afterwards (which implies that the scan-out buffer
was read back through the GTT and remained as the scan-out buffer with
the effect still apparent) - only the read back image is perfect and
shows no artefact. After a while (with no interaction and I think no
execution of batch buffers), the display magically returns to normal.

I'm open to suggestions as how to diagnose this problem and track down
the cause. Thanks.
-ickle


--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [Intel-gfx] [PATCH] i915: add page flipping ioctl

2009-02-20 Thread Barry Scott
I saw this and thought it was wrong.

int
 i915_seqno_passed(uint32_t seq1, uint32_t seq2)
 {
return (int32_t)(seq1 - seq2) = 0;

After a bit of thinking I realized that this is doing modulas arithmetic to
deal with the seqno wrapping around. Given its not obvious at first glance
you might like to add a comment that to explain?

Barry



--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [Intel-gfx] [PATCH] i915: add page flipping ioctl

2009-02-20 Thread Jesse Barnes
On Friday 20 February 2009 07:31:43 Barry Scott wrote:
 I saw this and thought it was wrong.

 int
  i915_seqno_passed(uint32_t seq1, uint32_t seq2)
  {
   return (int32_t)(seq1 - seq2) = 0;

 After a bit of thinking I realized that this is doing modulas arithmetic to
 deal with the seqno wrapping around. Given its not obvious at first glance
 you might like to add a comment that to explain?

It's a somewhat common idiom in the kernel, but yeah a comment wouldn't hurt.

-- 
Jesse Barnes, Intel Open Source Technology Center

--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[Bug 20235] New: [i915] [Regression] VT switch and suspend to disk broken

2009-02-20 Thread bugzilla-daemon
http://bugs.freedesktop.org/show_bug.cgi?id=20235

   Summary: [i915] [Regression] VT switch and suspend to disk broken
   Product: DRI
   Version: unspecified
  Platform: x86 (IA32)
OS/Version: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/Intel
AssignedTo: gordon@intel.com
ReportedBy: stava...@unina.it
CC: dri-devel@lists.sourceforge.net


Created an attachment (id=23139)
 -- (http://bugs.freedesktop.org/attachment.cgi?id=23139)
dmesg

My system:
00:02.0 VGA compatible controller: Intel Corporation Mobile GM965/GL960
Integrated Graphics Controller (rev 0c)

kernel: airlied's drm-fixed branch as of 2009-02-20 (commit:
**
author  etienne etienne.bas...@numericable.fr
   Thu, 19 Feb 2009 23:44:45 + (09:44 +1000)
 drm/radeon: update sarea copies of last_ variables on resume.
**
)

From debian experimental:
xserver 1.5.99.902
mesa 7.3
libdrm 2.4.4+git+20090205
intel 2.6.1

Acceleration: UXA

VT switch and suspend to disk are broken. Even restarting or shutting down from
X (KDE) makes the system freeze with a black screen.

Attached are dmesg taken (by ssh) after rebooting from KDE and Xorg.log.

According to the dmesg output, the problem seems to be:
BUG: unable to handle kernel NULL pointer dereference at 0050   
IP: [f87a009d] i915_gem_cleanup_hws+0x1d/0x70 [i915]  

I have used git-bisect and found that the first bad commit is:
**
author  Chris Wilson ch...@chris-wilson.co.uk
   Wed, 11 Feb 2009 14:52:44 + (14:52 +)
  commit  85a7bb98582b60b7e9130159d2464eb0bbac13f7
 drm/i915: Cleanup the hws on ringbuffer constrution failure.

 If we fail to create the ringbuffer, then we need to cleanup the allocated
 hws.
**

Reverting this single commit makes everything work again.


regards,
Stefano


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[Bug 20235] [i915] [Regression] VT switch and suspend to disk broken

2009-02-20 Thread bugzilla-daemon
http://bugs.freedesktop.org/show_bug.cgi?id=20235





--- Comment #1 from Stefano Avallone stava...@unina.it  2009-02-20 09:21:39 
PST ---
Created an attachment (id=23140)
 -- (http://bugs.freedesktop.org/attachment.cgi?id=23140)
Xorg log


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[PATCH] drm/i915: Fix regression in 95ca9d

2009-02-20 Thread Chris Wilson
The object is dereferenced before the NULL check. Oops.

Fixes http://bugs.freedesktop.org/show_bug.cgi?id=20235

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c |8 ++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a1c0950..1a1220d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3282,16 +3282,20 @@ static void
 i915_gem_cleanup_hws(struct drm_device *dev)
 {
drm_i915_private_t *dev_priv = dev-dev_private;
-   struct drm_gem_object *obj = dev_priv-hws_obj;
-   struct drm_i915_gem_object *obj_priv = obj-driver_private;
+   struct drm_gem_object *obj;
+   struct drm_i915_gem_object *obj_priv;
 
if (dev_priv-hws_obj == NULL)
return;
 
+   obj = dev_priv-hws_obj;
+   obj_priv = obj-driver_private;
+
kunmap(obj_priv-page_list[0]);
i915_gem_object_unpin(obj);
drm_gem_object_unreference(obj);
dev_priv-hws_obj = NULL;
+
memset(dev_priv-hws_map, 0, sizeof(dev_priv-hws_map));
dev_priv-hw_status_page = NULL;
 
-- 
1.6.0.4


--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


[Bug 20235] [i915] [Regression] VT switch and suspend to disk broken

2009-02-20 Thread bugzilla-daemon
http://bugs.freedesktop.org/show_bug.cgi?id=20235


Stefano Avallone stava...@unina.it changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||FIXED




--- Comment #2 from Stefano Avallone stava...@unina.it  2009-02-20 11:18:36 
PST ---
Fixed by the patch in:

http://lists.freedesktop.org/archives/intel-gfx/2009-February/001460.html


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel


Re: [Mesa3d-dev] DRI2 flush extension

2009-02-20 Thread Kristian Høgsberg
On Thu, Feb 19, 2009 at 8:46 PM, Alan Hourihane al...@fairlite.co.uk wrote:
 Attached is a new DRI2 flush extension that allows the driver to perform
 a real flush before dispatching a swap or Xserver copy operation.

 Currently we do this before a DRI2CopyRegion() call.

 This allows drivers a real end of scene flush to ensure rendering is
 complete prior to a swap.

 I've committed this already to the gallium-mesa-7.4 branch, but any
 comments appreciated before I push to the master branch?

Hi Alan,

A couple of comments below.

commit b163d4f9ee2ab4d54daf7c17c097cae51c9c6db2
Author: Alan Hourihane al...@vmware.com
Date:   Thu Feb 19 18:39:08 2009 +

glx: add support for a reallyFlush() function before swap occurs.

diff --git a/include/GL/internal/dri_interface.h
b/include/GL/internal/dri_interface.h
index 27cc1be..a726b93 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -78,6 +78,7 @@ typedef struct __DRIswrastExtensionRec
__DRIswrastExtension;
 typedef struct __DRIbufferRec  __DRIbuffer;
 typedef struct __DRIdri2ExtensionRec   __DRIdri2Extension;
 typedef struct __DRIdri2LoaderExtensionRec __DRIdri2LoaderExtension;
+typedef struct __DRI2flushExtensionRec __DRI2flushExtension;

 /*...@}*/

@@ -245,6 +246,16 @@ struct __DRItexBufferExtensionRec {
 __DRIdrawable *pDraw);
 };

+/**
+ * Used by drivers that implement DRI2
+ */
+#define __DRI2_FLUSH DRI2_Flush
+#define __DRI2_FLUSH_VERSION 1
+struct __DRI2flushExtensionRec {
+__DRIextension base;
+void (*flush)(__DRIdrawable *drawable);
+};
+

 /**
  * XML document describing the configuration options supported by the
diff --git a/src/glx/x11/dri2_glx.c b/src/glx/x11/dri2_glx.c
index 639aa19..fdda852 100644
--- a/src/glx/x11/dri2_glx.c
+++ b/src/glx/x11/dri2_glx.c
@@ -207,7 +207,13 @@ static void dri2CopySubBuffer(__GLXDRIdrawable *pdraw,
 xrect.width = width;
 xrect.height = height;

+#ifdef __DRI2_FLUSH
+if (pdraw-psc-f)
+   (*pdraw-psc-f-flush)(pdraw-driDrawable);
+#endif
+
 region = XFixesCreateRegion(pdraw-psc-dpy, xrect, 1);
+/* should get a fence ID back from here at some point */
 DRI2CopyRegion(pdraw-psc-dpy, pdraw-drawable, region,
   DRI2BufferFrontLeft, DRI2BufferBackLeft);
 XFixesDestroyRegion(pdraw-psc-dpy, region);
@@ -235,6 +241,11 @@ static void dri2WaitX(__GLXDRIdrawable *pdraw)
 xrect.width = priv-width;
 xrect.height = priv-height;

+#ifdef __DRI2_FLUSH
+if (pdraw-psc-f)
+   (*pdraw-psc-f-flush)(pdraw-driDrawable);
+#endif
+

This flush call isn't necessary - glXWaitX() is called to wait for X
rendering to the drawable to finish so there can't be any unflushed
DRI driver activity.

 region = XFixesCreateRegion(pdraw-psc-dpy, xrect, 1);
 DRI2CopyRegion(pdraw-psc-dpy, pdraw-drawable, region,
   DRI2BufferFakeFrontLeft, DRI2BufferFrontLeft);
@@ -255,6 +266,11 @@ static void dri2WaitGL(__GLXDRIdrawable *pdraw)
 xrect.width = priv-width;
 xrect.height = priv-height;

+#ifdef __DRI2_FLUSH
+if (pdraw-psc-f)
+   (*pdraw-psc-f-flush)(pdraw-driDrawable);
+#endif
+
 region = XFixesCreateRegion(pdraw-psc-dpy, xrect, 1);
 DRI2CopyRegion(pdraw-psc-dpy, pdraw-drawable, region,
   DRI2BufferFrontLeft, DRI2BufferFakeFrontLeft);
diff --git a/src/glx/x11/dri_common.c b/src/glx/x11/dri_common.c
index 4fda649..90c3d8c 100644
--- a/src/glx/x11/dri_common.c
+++ b/src/glx/x11/dri_common.c
@@ -392,6 +392,13 @@ driBindExtensions(__GLXscreenConfigs *psc, int dri2)
}
 #endif

+#ifdef __DRI2_FLUSH
+   if ((strcmp(extensions[i]-name, __DRI2_FLUSH) == 0)  dri2) {
+   psc-f = (__DRI2flushExtension *) extensions[i];
+   /* internal driver extension, no GL extension exposed */
+   }
+#endif

The driver needs to know whether the loader is new enough that it
supports the flush extension so that it can enable the lazy glFlush()
optiomization.  An older loader will just ignore the flush extension
and thus never call that entry point, in which case the driver must
flush on every glFlush() call.  The only way to do this I can think of
right now is to add an 'enable()' entrypoint to the extension to tell
the driver that the loader will call -flush() and it's ok to ignore
glFlush().

/* Ignore unknown extensions */
 }
 }
diff --git a/src/glx/x11/glxclient.h b/src/glx/x11/glxclient.h
index 3e70759..caf58bb 100644
--- a/src/glx/x11/glxclient.h
+++ b/src/glx/x11/glxclient.h
@@ -519,6 +519,10 @@ struct __GLXscreenConfigsRec {
 const __DRItexBufferExtension *texBuffer;
 #endif

+#ifdef __DRI2_FLUSH
+const __DRI2flushExtension *f;
+#endif
+
 #endif

 /**

--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the 

Re: [PATCH] drm: Take mmap_sem up front to avoid lock order violations.

2009-02-20 Thread Eric Anholt
On Thu, 2009-02-19 at 13:57 +0100, Nick Piggin wrote:
 On Thu, Feb 19, 2009 at 10:19:05AM +0100, Peter Zijlstra wrote:
  On Wed, 2009-02-18 at 11:38 -0500, k...@bitplanet.net wrote:
   From: Kristian Høgsberg k...@redhat.com
   
   A number of GEM operations (and legacy drm ones) want to copy data to
   or from userspace while holding the struct_mutex lock.  However, the
   fault handler calls us with the mmap_sem held and thus enforces the
   opposite locking order.  This patch downs the mmap_sem up front for
   those operations that access userspace data under the struct_mutex
   lock to ensure the locking order is consistent.
   
   Signed-off-by: Kristian Høgsberg k...@redhat.com
   ---
   
   Here's a different and simpler attempt to fix the locking order
   problem.  We can just down_read() the mmap_sem pre-emptively up-front,
   and the locking order is respected.  It's simpler than the
   mutex_trylock() game, avoids introducing a new mutex.
 
 The simple way to fix this is to just allocate a temporary buffer
 to copy a snapshot of the data going to/from userspace. Then do the
 real usercopy to/from that buffer outside the locks.
 
 You don't have any performance critical bulk copies (ie. that will
 blow the L1 cache), do you? 

16kb is the most common size (batchbuffers).  32k is popular on 915
(vertex), and varying between 0-128k on 965 (vertex).  The pwrite path
generally represents 10-30% of CPU consumption in CPU-bound apps.

-- 
Eric Anholt
e...@anholt.net eric.anh...@intel.com




signature.asc
Description: This is a digitally signed message part
--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel