From f2653134995c5a4b918eb1ed5ac09efb35acf77a Mon Sep 17 00:00:00 2001 From: Boram Park Date: Sat, 9 Jul 2016 17:58:04 +0900 Subject: [PATCH] Use timer to call the output change callback of the sub-thread. The output change callback of tdm_server and tdm_vblank was called in the main thread. And it made the multi thread issue. If we use the timer, we can call the sub-thread's output change callback in sub-thread. And, after tdm_event_loop_deinit, we don't worry about thread things because it's finalized. Change-Id: I69013b302673551b887384325b14c94e3a53646c --- src/tdm.c | 11 +++++-- src/tdm_display.c | 82 ++++++++++++++++++++++------------------------------ src/tdm_event_loop.c | 6 ++++ src/tdm_macro.h | 2 +- src/tdm_private.h | 15 +++++----- src/tdm_server.c | 1 + src/tdm_thread.c | 10 +++++++ src/tdm_vblank.c | 32 +++++++++++--------- 8 files changed, 87 insertions(+), 72 deletions(-) diff --git a/src/tdm.c b/src/tdm.c index 9653d8f..8a74ae2 100644 --- a/src/tdm.c +++ b/src/tdm.c @@ -185,6 +185,9 @@ _tdm_display_destroy_private_output(tdm_private_output *private_output) _tdm_display_destroy_caps_output(&private_output->caps); + if (private_output->dpms_changed_timer) + tdm_event_loop_source_remove(private_output->dpms_changed_timer); + private_output->stamp = 0; free(private_output); } @@ -993,9 +996,13 @@ tdm_display_deinit(tdm_display *dpy) return; } + /* dont move the position of lock/unlock. all resource should be protected + * during destroying. after tdm_event_loop_deinit, we don't worry about thread + * things because it's finalized. + */ _pthread_mutex_lock(&private_display->lock); - tdm_event_loop_deinit(private_display); + _pthread_mutex_unlock(&private_display->lock); _tdm_display_destroy_private_display(private_display); _tdm_display_unload_module(private_display); @@ -1007,8 +1014,6 @@ tdm_display_deinit(tdm_display *dpy) tdm_helper_set_fd("TDM_DRM_MASTER_FD", -1); - _pthread_mutex_unlock(&private_display->lock); - pthread_mutex_destroy(&private_display->lock); free(private_display); g_private_display = NULL; diff --git a/src/tdm_display.c b/src/tdm_display.c index c3bd904..aa29226 100644 --- a/src/tdm_display.c +++ b/src/tdm_display.c @@ -102,46 +102,6 @@ private_output = private_layer->private_output; \ private_display = private_output->private_display -INTERN tdm_error -_tdm_display_lock(tdm_display *dpy, const char *func) -{ - tdm_private_display *private_display = (tdm_private_display*)dpy; - int ret; - - if (tdm_debug_mutex) - TDM_INFO("mutex lock: %s", func); - - ret = pthread_mutex_trylock(&private_display->lock); - if (ret < 0) { - if (ret == EBUSY) - TDM_ERR("mutex lock busy: %s", func); - else - TDM_ERR("mutex lock failed: %s(%m)", func); - return TDM_ERROR_OPERATION_FAILED; - } - - pthread_mutex_lock(&tdm_mutex_check_lock); - tdm_mutex_locked = 1; - pthread_mutex_unlock(&tdm_mutex_check_lock); - - return TDM_ERROR_NONE; -} - -INTERN void -_tdm_display_unlock(tdm_display *dpy, const char *func) -{ - tdm_private_display *private_display = (tdm_private_display*)dpy; - - if (tdm_debug_mutex) - TDM_INFO("mutex unlock: %s", func); - - pthread_mutex_lock(&tdm_mutex_check_lock); - tdm_mutex_locked = 0; - pthread_mutex_unlock(&tdm_mutex_check_lock); - - pthread_mutex_unlock(&private_display->lock); -} - EXTERN tdm_error tdm_display_get_capabilities(tdm_display *dpy, tdm_display_capability *capabilities) @@ -940,10 +900,10 @@ tdm_output_wait_vblank(tdm_output *output, int interval, int sync, _pthread_mutex_lock(&private_display->lock); if (private_output->current_dpms_value > TDM_OUTPUT_DPMS_ON) { - TDM_ERR("output(%d) dpms: %s", private_output->pipe, + TDM_WRN("output(%d) dpms: %s", private_output->pipe, tdm_dpms_str(private_output->current_dpms_value)); _pthread_mutex_unlock(&private_display->lock); - return TDM_ERROR_BAD_REQUEST; + return TDM_ERROR_DPMS_OFF; } func_output = &private_display->func_output; @@ -1037,7 +997,7 @@ tdm_output_commit(tdm_output *output, int sync, tdm_output_commit_handler func, TDM_ERR("output(%d) dpms: %s", private_output->pipe, tdm_dpms_str(private_output->current_dpms_value)); _pthread_mutex_unlock(&private_display->lock); - return TDM_ERROR_BAD_REQUEST; + return TDM_ERROR_DPMS_OFF; } ret = _tdm_output_commit(output, sync, func, user_data); @@ -1097,6 +1057,21 @@ tdm_output_get_mode(tdm_output *output, const tdm_output_mode **mode) return ret; } +static tdm_error +_tdm_output_dpms_changed_timeout(void *user_data) +{ + tdm_private_output *private_output = user_data; + tdm_value value; + + value.u32 = private_output->current_dpms_value; + tdm_output_call_change_handler_internal(private_output, + &private_output->change_handler_list_sub, + TDM_OUTPUT_CHANGE_DPMS, + value, 0); + + return TDM_ERROR_NONE; +} + EXTERN tdm_error tdm_output_set_dpms(tdm_output *output, tdm_output_dpms dpms_value) { @@ -1113,6 +1088,17 @@ tdm_output_set_dpms(tdm_output *output, tdm_output_dpms dpms_value) return TDM_ERROR_NONE; } + if (!private_output->dpms_changed_timer) { + private_output->dpms_changed_timer = + tdm_event_loop_add_timer_handler(private_output->private_display, + _tdm_output_dpms_changed_timeout, private_output, NULL); + if (!private_output->dpms_changed_timer) { + TDM_ERR("can't create dpms timer!!"); + _pthread_mutex_unlock(&private_display->lock); + return TDM_ERROR_OUT_OF_MEMORY; + } + } + func_output = &private_display->func_output; if (!func_output->output_set_dpms) { @@ -1134,11 +1120,11 @@ tdm_output_set_dpms(tdm_output *output, tdm_output_dpms dpms_value) TDM_OUTPUT_CHANGE_DPMS, value, 0); - //TODO: safe? We can't take care of the user_data of callback functions. - tdm_output_call_change_handler_internal(private_output, - &private_output->change_handler_list_sub, - TDM_OUTPUT_CHANGE_DPMS, - value, 1); + if (!LIST_IS_EMPTY(&private_output->change_handler_list_sub)) { + ret = tdm_event_loop_source_timer_update(private_output->dpms_changed_timer, 1); + if (ret != TDM_ERROR_NONE) + TDM_NEVER_GET_HERE(); + } } _pthread_mutex_unlock(&private_display->lock); diff --git a/src/tdm_event_loop.c b/src/tdm_event_loop.c index 51a8f55..c068c09 100644 --- a/src/tdm_event_loop.c +++ b/src/tdm_event_loop.c @@ -147,7 +147,11 @@ tdm_event_loop_deinit(tdm_private_display *private_display) if (!private_display->private_loop) return; + /* after tdm_thread_deinit, we don't worry about thread things because it's finalized */ tdm_thread_deinit(private_display->private_loop); + + + _pthread_mutex_unlock(&private_display->lock); tdm_server_deinit(private_display->private_loop); if (private_display->private_loop->backend_source) @@ -158,6 +162,8 @@ tdm_event_loop_deinit(tdm_private_display *private_display) free(private_display->private_loop); private_display->private_loop = NULL; + + _pthread_mutex_lock(&private_display->lock); } INTERN void diff --git a/src/tdm_macro.h b/src/tdm_macro.h index 659ead3..3983c68 100644 --- a/src/tdm_macro.h +++ b/src/tdm_macro.h @@ -102,7 +102,7 @@ extern "C" { } \ } -#define TDM_NEVER_GET_HERE() TDM_ERR("** NEVER GET HERE **") +#define TDM_NEVER_GET_HERE() TDM_WRN("** NEVER GET HERE **") #define TDM_SNPRINTF(p, len, fmt, ARG...) \ do { \ diff --git a/src/tdm_private.h b/src/tdm_private.h index 05fb7a9..a9ad25e 100644 --- a/src/tdm_private.h +++ b/src/tdm_private.h @@ -170,6 +170,9 @@ struct _tdm_private_output { struct list_head change_handler_list_sub; void **layers_ptr; + + /* TODO: temp solution for handling DPMS things in sub-htread */ + tdm_event_loop_source *dpms_changed_timer; }; struct _tdm_private_layer { @@ -495,19 +498,17 @@ extern int tdm_dump_enable; static inline int TDM_MUTEX_IS_LOCKED(void) { int ret; + /* if thread is not running, we don't need to consider mutex things. */ + if (!tdm_thread_is_running()) + return 1; pthread_mutex_lock(&tdm_mutex_check_lock); ret = (tdm_mutex_locked == 1); pthread_mutex_unlock(&tdm_mutex_check_lock); return ret; } -tdm_error -_tdm_display_lock(tdm_display *dpy, const char *func); -void -_tdm_display_unlock(tdm_display *dpy, const char *func); - -#define tdm_display_lock(dpy) _tdm_display_lock(dpy, __FUNCTION__) -#define tdm_display_unlock(dpy) _tdm_display_unlock(dpy, __FUNCTION__) +#define tdm_display_lock(dpy) _pthread_mutex_lock(&((tdm_private_display *)dpy)->lock) +#define tdm_display_unlock(dpy) _pthread_mutex_unlock(&((tdm_private_display *)dpy)->lock) tdm_error tdm_display_update_output(tdm_private_display *private_display, diff --git a/src/tdm_server.c b/src/tdm_server.c index bb8ff6c..7c18228 100644 --- a/src/tdm_server.c +++ b/src/tdm_server.c @@ -48,6 +48,7 @@ * However, the internal function which does lock/unlock the mutex of * private_display in itself can be called. * - DO NOT use the tdm_private_display structure here. + * - The callback function things can be called in main thread. */ struct _tdm_private_server { diff --git a/src/tdm_thread.c b/src/tdm_thread.c index 3ae6d0b..1a9a853 100644 --- a/src/tdm_thread.c +++ b/src/tdm_thread.c @@ -173,13 +173,23 @@ tdm_thread_init(tdm_private_loop *private_loop) INTERN void tdm_thread_deinit(tdm_private_loop *private_loop) { + tdm_private_display *private_display; + TDM_RETURN_IF_FAIL(TDM_MUTEX_IS_LOCKED()); if (!private_loop->private_thread) return; pthread_cancel(private_loop->private_thread->event_thread); + + private_display = private_loop->dpy; + + /* before falling into the block of pthread_join, we have to unlock the mutex + * for subthread to use the mutex. + */ + _pthread_mutex_unlock(&private_display->lock); pthread_join(private_loop->private_thread->event_thread, NULL); + _pthread_mutex_lock(&private_display->lock); if (private_loop->private_thread->pipe[0] >= 0) close(private_loop->private_thread->pipe[0]); diff --git a/src/tdm_vblank.c b/src/tdm_vblank.c index 4af896f..30ed7df 100644 --- a/src/tdm_vblank.c +++ b/src/tdm_vblank.c @@ -42,11 +42,9 @@ #include "tdm_list.h" /* CAUTION: - * - tdm vblank doesn't care about thread things. - * - DO NOT use the TDM internal functions here. - * However, the internal function which does lock/unlock the mutex of - * private_display in itself can be called. - * - DO NOT use the tdm_private_display structure here. + * tdm vblank doesn't care about thread things. + * However, to use tdm_event_loop_xxx functions, have to use the internal function. + * So need to lock/unlock the mutex of private_display. */ /* TDM vblank @@ -362,10 +360,9 @@ tdm_vblank_destroy(tdm_vblank *vblank) #endif if (private_vblank->SW_timer) { - if (tdm_display_lock(private_vblank->dpy) == TDM_ERROR_NONE) { - tdm_event_loop_source_remove(private_vblank->SW_timer); - tdm_display_unlock(private_vblank->dpy); - } + tdm_display_lock(private_vblank->dpy); + tdm_event_loop_source_remove(private_vblank->SW_timer); + tdm_display_unlock(private_vblank->dpy); } tdm_output_remove_change_handler(private_vblank->output, @@ -495,7 +492,7 @@ _tdm_vblank_wait_HW(tdm_vblank_wait_info *wait_info) _tdm_vblank_cb_vblank_HW, wait_info); if (ret != TDM_ERROR_NONE) { - VER("wait(%p) failed", wait_info); + VWR("wait(%p) failed", wait_info); LIST_DEL(&wait_info->link); return ret; } @@ -627,8 +624,7 @@ _tdm_vblank_sw_timer_update(tdm_private_vblank *private_vblank) VDB("wait(%p) curr(%4lu) target(%4lu) ms_delay(%d)", first_wait_info, curr, target, ms_delay); - ret = tdm_display_lock(private_vblank->dpy); - TDM_RETURN_VAL_IF_FAIL(ret == TDM_ERROR_NONE, ret); + tdm_display_lock(private_vblank->dpy); if (!private_vblank->SW_timer) { private_vblank->SW_timer = @@ -745,6 +741,10 @@ _tdm_vblank_wait_SW(tdm_vblank_wait_info *wait_info) if (do_wait) { ret = tdm_output_wait_vblank(private_vblank->output, 1, 0, _tdm_vblank_cb_vblank_SW_first, wait_info); + if (ret == TDM_ERROR_DPMS_OFF) { + TDM_WRN("use SW"); + goto use_sw; + } if (ret != TDM_ERROR_NONE) { LIST_DEL(&wait_info->link); return ret; @@ -754,6 +754,7 @@ _tdm_vblank_wait_SW(tdm_vblank_wait_info *wait_info) return TDM_ERROR_NONE; } +use_sw: TDM_RETURN_VAL_IF_FAIL(wait_info->target_sec > 0, TDM_ERROR_OPERATION_FAILED); _tdm_vblank_insert_wait(wait_info, &private_vblank->SW_wait_list); @@ -892,8 +893,13 @@ tdm_vblank_wait(tdm_vblank *vblank, unsigned int req_sec, unsigned int req_usec, _tdm_vblank_calculate_target(wait_info); - if (private_vblank->HW_enable) + if (private_vblank->HW_enable) { ret = _tdm_vblank_wait_HW(wait_info); + if (ret == TDM_ERROR_DPMS_OFF) { + TDM_WRN("try to use SW"); + ret = _tdm_vblank_wait_SW(wait_info); + } + } else ret = _tdm_vblank_wait_SW(wait_info); -- 2.7.4