From 128feef40e81b320af59edc7a2fcf6b2ad8d2b8d Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Fri, 20 May 2016 14:16:18 -0600 Subject: st/wgl: refactor framebuffer locking code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the old stw_framebuffer_reference() function into two new functions: stw_framebuffer_reference_locked() which increments the refcount and stw_framebuffer_release_locked() which decrements the refcount and destroys the buffer when the count hits zero. Original patch by Jose. Modified by Brian (clean-ups, lock assertion checks, etc). Reviewed-by: José Fonseca --- src/gallium/state_trackers/wgl/stw_context.c | 51 ++++++++++++++++-------- src/gallium/state_trackers/wgl/stw_device.h | 4 ++ src/gallium/state_trackers/wgl/stw_framebuffer.c | 43 +++++--------------- src/gallium/state_trackers/wgl/stw_framebuffer.h | 21 +++++++++- 4 files changed, 68 insertions(+), 51 deletions(-) (limited to 'src/gallium/state_trackers/wgl') diff --git a/src/gallium/state_trackers/wgl/stw_context.c b/src/gallium/state_trackers/wgl/stw_context.c index 9971f95..b1e5f5e 100644 --- a/src/gallium/state_trackers/wgl/stw_context.c +++ b/src/gallium/state_trackers/wgl/stw_context.c @@ -390,7 +390,6 @@ stw_make_current(HDC hdc, DHGLRC dhglrc) { struct stw_context *old_ctx = NULL; struct stw_context *ctx = NULL; - struct stw_framebuffer *fb = NULL; BOOL ret = FALSE; if (!stw_dev) @@ -409,6 +408,7 @@ stw_make_current(HDC hdc, DHGLRC dhglrc) } if (dhglrc) { + struct stw_framebuffer *fb = NULL; stw_lock_contexts(stw_dev); ctx = stw_lookup_context_locked( dhglrc ); stw_unlock_contexts(stw_dev); @@ -416,6 +416,7 @@ stw_make_current(HDC hdc, DHGLRC dhglrc) goto fail; } + /* This call locks fb's mutex */ fb = stw_framebuffer_from_hdc( hdc ); if (fb) { stw_framebuffer_update(fb); @@ -434,6 +435,7 @@ stw_make_current(HDC hdc, DHGLRC dhglrc) } if (fb->iPixelFormat != ctx->iPixelFormat) { + stw_framebuffer_unlock(fb); SetLastError(ERROR_INVALID_PIXEL_FORMAT); goto fail; } @@ -441,36 +443,53 @@ stw_make_current(HDC hdc, DHGLRC dhglrc) /* Bind the new framebuffer */ ctx->hdc = hdc; + struct stw_framebuffer *old_fb = ctx->current_framebuffer; + if (old_fb != fb) { + stw_framebuffer_reference_locked(fb); + ctx->current_framebuffer = fb; + } + stw_framebuffer_unlock(fb); + /* Note: when we call this function we will wind up in the * stw_st_framebuffer_validate_locked() function which will incur * a recursive fb->mutex lock. */ ret = stw_dev->stapi->make_current(stw_dev->stapi, ctx->st, fb->stfb, fb->stfb); - stw_framebuffer_reference(&ctx->current_framebuffer, fb); - } else { - ret = stw_dev->stapi->make_current(stw_dev->stapi, NULL, NULL, NULL); - } -fail: + if (old_fb && old_fb != fb) { + stw_lock_framebuffers(stw_dev); + stw_framebuffer_lock(old_fb); + stw_framebuffer_release_locked(old_fb); + stw_unlock_framebuffers(stw_dev); + } - if (fb) { - stw_framebuffer_unlock(fb); - } +fail: + /* fb must be unlocked at this point. */ + assert(!stw_own_mutex(&fb->mutex)); - /* On failure, make the thread's current rendering context not current - * before returning. - */ - if (!ret) { - stw_dev->stapi->make_current(stw_dev->stapi, NULL, NULL, NULL); - ctx = NULL; + /* On failure, make the thread's current rendering context not current + * before returning. + */ + if (!ret) { + stw_make_current(NULL, 0); + } + } else { + ret = stw_dev->stapi->make_current(stw_dev->stapi, NULL, NULL, NULL); } /* Unreference the previous framebuffer if any. It must be done after * make_current, as it can be referenced inside. */ if (old_ctx && old_ctx != ctx) { - stw_framebuffer_reference(&old_ctx->current_framebuffer, NULL); + struct stw_framebuffer *old_fb = old_ctx->current_framebuffer; + if (old_fb) { + old_ctx->current_framebuffer = NULL; + stw_lock_framebuffers(stw_dev); + stw_framebuffer_lock(old_fb); + stw_framebuffer_release_locked(old_fb); + stw_unlock_framebuffers(stw_dev); + } } return ret; diff --git a/src/gallium/state_trackers/wgl/stw_device.h b/src/gallium/state_trackers/wgl/stw_device.h index 3f0dffe..15d66a2 100644 --- a/src/gallium/state_trackers/wgl/stw_device.h +++ b/src/gallium/state_trackers/wgl/stw_device.h @@ -67,6 +67,10 @@ struct stw_device CRITICAL_SECTION ctx_mutex; struct handle_table *ctx_table; + /* TODO: use an atomic counter to track the number of locked + * stw_framebuffer objects. Assert that the counter is zero when + * trying to lock this mutex. + */ CRITICAL_SECTION fb_mutex; struct stw_framebuffer *fb_head; diff --git a/src/gallium/state_trackers/wgl/stw_framebuffer.c b/src/gallium/state_trackers/wgl/stw_framebuffer.c index b49bc22..1263615 100644 --- a/src/gallium/state_trackers/wgl/stw_framebuffer.c +++ b/src/gallium/state_trackers/wgl/stw_framebuffer.c @@ -67,14 +67,18 @@ stw_framebuffer_from_hwnd_locked(HWND hwnd) * Decrement the reference count on the given stw_framebuffer object. * If the reference count hits zero, destroy the object. * - * Note: Both stw_dev::fb_mutex and stw_framebuffer::mutex must already - * be locked. + * Note: Both stw_dev::fb_mutex and stw_framebuffer::mutex must already be + * locked. After this function completes, the fb's mutex will be unlocked. */ -static void -stw_framebuffer_destroy_locked(struct stw_framebuffer *fb) +void +stw_framebuffer_release_locked(struct stw_framebuffer *fb) { struct stw_framebuffer **link; + assert(fb); + assert(stw_own_mutex(&fb->mutex)); + assert(stw_own_mutex(&stw_dev->fb_mutex)); + /* check the reference count */ fb->refcnt--; if (fb->refcnt) { @@ -223,7 +227,7 @@ stw_call_window_proc(int nCode, WPARAM wParam, LPARAM lParam) stw_lock_framebuffers(stw_dev); fb = stw_framebuffer_from_hwnd_locked( pParams->hwnd ); if (fb) - stw_framebuffer_destroy_locked(fb); + stw_framebuffer_release_locked(fb); stw_unlock_framebuffers(stw_dev); } @@ -304,33 +308,6 @@ stw_framebuffer_create(HDC hdc, int iPixelFormat) /** - * Have ptr reference fb. The referenced framebuffer should be locked. - */ -void -stw_framebuffer_reference(struct stw_framebuffer **ptr, - struct stw_framebuffer *fb) -{ - struct stw_framebuffer *old_fb = *ptr; - - if (old_fb == fb) - return; - - if (fb) - fb->refcnt++; - if (old_fb) { - stw_lock_framebuffers(stw_dev); - - stw_framebuffer_lock(old_fb); - stw_framebuffer_destroy_locked(old_fb); - - stw_unlock_framebuffers(stw_dev); - } - - *ptr = fb; -} - - -/** * Update the framebuffer's size if necessary. */ void @@ -369,7 +346,7 @@ stw_framebuffer_cleanup(void) next = fb->next; stw_framebuffer_lock(fb); - stw_framebuffer_destroy_locked(fb); + stw_framebuffer_release_locked(fb); fb = next; } diff --git a/src/gallium/state_trackers/wgl/stw_framebuffer.h b/src/gallium/state_trackers/wgl/stw_framebuffer.h index 0e2c61f..029fb9f 100644 --- a/src/gallium/state_trackers/wgl/stw_framebuffer.h +++ b/src/gallium/state_trackers/wgl/stw_framebuffer.h @@ -34,6 +34,7 @@ #include #include "util/u_debug.h" +#include "stw_st.h" struct pipe_resource; @@ -131,9 +132,24 @@ struct stw_framebuffer struct stw_framebuffer * stw_framebuffer_create(HDC hdc, int iPixelFormat); + +/** + * Increase fb reference count. The referenced framebuffer should be locked. + * + * It's not necessary to hold stw_dev::fb_mutex global lock. + */ +static inline void +stw_framebuffer_reference_locked(struct stw_framebuffer *fb) +{ + if (fb) { + assert(stw_own_mutex(&fb->mutex)); + fb->refcnt++; + } +} + + void -stw_framebuffer_reference(struct stw_framebuffer **ptr, - struct stw_framebuffer *fb); +stw_framebuffer_release_locked(struct stw_framebuffer *fb); /** * Search a framebuffer with a matching HWND. @@ -179,6 +195,7 @@ static inline void stw_framebuffer_unlock(struct stw_framebuffer *fb) { assert(fb); + assert(stw_own_mutex(&fb->mutex)); LeaveCriticalSection(&fb->mutex); } -- cgit v1.1