Re: [PATCH] drm: Fix lock order reversal between mmap_sem and struct_mutex.
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.
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.
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.
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.
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
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
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
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
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
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
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
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
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
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
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.
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