Use timer to call the output change callback of the sub-thread. 32/79232/3
authorBoram Park <boram1288.park@samsung.com>
Sat, 9 Jul 2016 08:58:04 +0000 (17:58 +0900)
committerBoram Park <boram1288.park@samsung.com>
Sat, 9 Jul 2016 09:14:42 +0000 (18:14 +0900)
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
src/tdm_display.c
src/tdm_event_loop.c
src/tdm_macro.h
src/tdm_private.h
src/tdm_server.c
src/tdm_thread.c
src/tdm_vblank.c

index 9653d8f..8a74ae2 100644 (file)
--- 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;
index c3bd904..aa29226 100644 (file)
        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);
index 51a8f55..c068c09 100644 (file)
@@ -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
index 659ead3..3983c68 100644 (file)
@@ -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 { \
index 05fb7a9..a9ad25e 100644 (file)
@@ -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,
index bb8ff6c..7c18228 100644 (file)
@@ -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 {
index 3ae6d0b..1a9a853 100644 (file)
@@ -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]);
index 4af896f..30ed7df 100644 (file)
 #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);