From d7a2a7c4c120fd94b91898ed18ed096005a53596 Mon Sep 17 00:00:00 2001 From: vanfanel Date: Thu, 19 Mar 2015 12:23:18 +0100 Subject: [PATCH] Fixed lockup issue on the dispmanx driver. Rewrote some parts for simplicity. --- gfx/drivers/dispmanx_gfx.c | 399 ++++++++++++++----------------------- 1 file changed, 146 insertions(+), 253 deletions(-) diff --git a/gfx/drivers/dispmanx_gfx.c b/gfx/drivers/dispmanx_gfx.c index f3739bd0de..021c5f2ea3 100644 --- a/gfx/drivers/dispmanx_gfx.c +++ b/gfx/drivers/dispmanx_gfx.c @@ -50,7 +50,6 @@ struct dispmanx_video DISPMANX_DISPLAY_HANDLE_T display; DISPMANX_MODEINFO_T amode; DISPMANX_UPDATE_HANDLE_T update; - DISPMANX_RESOURCE_HANDLE_T resources[NUMPAGES]; DISPMANX_ELEMENT_HANDLE_T element; VC_IMAGE_TYPE_T pixFormat; VC_DISPMANX_ALPHA_T *alpha; @@ -74,7 +73,8 @@ struct dispmanx_video struct dispmanx_page *pages; struct dispmanx_page *currentPage; - unsigned int pageflip_pending; + struct dispmanx_page *nextPage; + bool pageflip_pending; /* For console blanking */ int fd; @@ -86,13 +86,10 @@ struct dispmanx_video scond_t *vsync_condition; slock_t *pending_mutex; - /* Mutex to isolate the vsync condition signaling */ - slock_t *vsync_cond_mutex; - /* Menu */ bool menu_active; DISPMANX_ELEMENT_HANDLE_T menu_element; - DISPMANX_RESOURCE_HANDLE_T menu_resources[2]; + DISPMANX_RESOURCE_HANDLE_T menu_resource; VC_IMAGE_TYPE_T menu_pixFormat; VC_DISPMANX_ALPHA_T *menu_alpha; VC_RECT_T menu_srcRect; @@ -109,16 +106,7 @@ struct dispmanx_video struct dispmanx_page { - unsigned int numpage; - struct dispmanx_page *next; - /* This field will allow us to - * access the main _dispvars struct - * from the vsync CB function */ - struct dispmanx_video *dispvars; - /* Each page has it's own mutex for - * isolating it's used flag access. */ - slock_t *page_used_mutex; - bool used; + DISPMANX_RESOURCE_HANDLE_T resource; }; static void dispmanx_blank_console(void *data) @@ -182,104 +170,25 @@ static void dispmanx_unblank_console(void *data) system("setterm -cursor on"); } -/* Find a free page, clear it if necessary, and return the page. - * - * If no free page is available when called, wait for a page flip. - */ -static struct dispmanx_page *dispmanx_get_free_page(void* data) -{ - unsigned i; - struct dispmanx_video *_dispvars = data; - struct dispmanx_page *page = NULL; - - while (!page) - { - /* Try to find a free page */ - for (i = 0; i < NUMPAGES; ++i) - { - if (!_dispvars->pages[i].used) - { - page = (_dispvars->pages) + i; - break; - } - } - - /* If no page is free at the moment, - * wait until a free page is freed by vsync CB. */ - if (!page) - { - slock_lock(_dispvars->vsync_cond_mutex); - scond_wait(_dispvars->vsync_condition, _dispvars->vsync_cond_mutex); - slock_unlock(_dispvars->vsync_cond_mutex); - } - } - - slock_lock(page->page_used_mutex); - page->used = true; - slock_unlock(page->page_used_mutex); - - return page; -} - static void vsync_callback(DISPMANX_UPDATE_HANDLE_T u, void *data) -{ - struct dispmanx_page *page = data; - - if (!page) - return; - - /* We signal the vsync condition, just in case - * we're waiting for it somewhere (no free pages, etc). */ - slock_lock(page->dispvars->vsync_cond_mutex); - scond_signal(page->dispvars->vsync_condition); - slock_unlock(page->dispvars->vsync_cond_mutex); - - slock_lock(page->dispvars->pending_mutex); - page->dispvars->pageflip_pending--; - slock_unlock(page->dispvars->pending_mutex); - - if (page->dispvars->currentPage) - { - slock_lock(page->dispvars->currentPage->page_used_mutex); - - /* We mark as free the page that was visible until now */ - page->dispvars->currentPage->used = false; - - slock_unlock(page->dispvars->currentPage->page_used_mutex); - } - - /* The page on which we just issued the flip that - * caused this callback becomes the visible one */ - page->dispvars->currentPage = page; -} - -static void dispmanx_flip(struct dispmanx_page *page, void *data) { struct dispmanx_video *_dispvars = data; - if (!_dispvars) - return; + /* Changing the page to write must be done before the signaling + * so we have the right page in nextPage when update_main continues */ - /* Dispmanx doesn't support issuing more than one pageflip. - * If we do, the second CB isn't called. */ - if (_dispvars->pageflip_pending > 0) - { - slock_lock(_dispvars->vsync_cond_mutex); - scond_wait(_dispvars->vsync_condition, _dispvars->vsync_cond_mutex); - slock_unlock(_dispvars->vsync_cond_mutex); - } - - /* Issue a page flip at the next vblank interval - * (will be done at vsync anyway). */ - _dispvars->update = vc_dispmanx_update_start(0); - - vc_dispmanx_element_change_source(_dispvars->update, _dispvars->element, - _dispvars->resources[page->numpage]); - - vc_dispmanx_update_submit(_dispvars->update, vsync_callback, (void*)page); + if (_dispvars->nextPage == &_dispvars->pages[0]) + _dispvars->nextPage = &_dispvars->pages[1]; + else + _dispvars->nextPage = &_dispvars->pages[0]; + /* These two things must be isolated "atomically" to avoid getting + * a false positive in the pending_mutex test in update_main. */ slock_lock(_dispvars->pending_mutex); - _dispvars->pageflip_pending++; + + _dispvars->pageflip_pending = false; + scond_signal(_dispvars->vsync_condition); + slock_unlock(_dispvars->pending_mutex); } @@ -294,7 +203,7 @@ static void dispmanx_free_main_resources(void *data) _dispvars->update = vc_dispmanx_update_start(0); for (i = 0; i < NUMPAGES; i++) - vc_dispmanx_resource_delete(_dispvars->resources[i]); + vc_dispmanx_resource_delete(_dispvars->pages[i].resource); vc_dispmanx_element_remove(_dispvars->update, _dispvars->element); vc_dispmanx_update_submit_sync(_dispvars->update); @@ -303,31 +212,31 @@ static void dispmanx_free_main_resources(void *data) static bool dispmanx_setup_scale(void *data, unsigned width, unsigned height, unsigned pitch) { - int i, dst_ypos; - VC_DISPMANX_ALPHA_T layerAlpha; - struct dispmanx_video *_dispvars = data; + int i, dst_ypos; + VC_DISPMANX_ALPHA_T layerAlpha; + struct dispmanx_video *_dispvars = data; if (!_dispvars) return false; - /* Since internal frame resolution seems to have changed, we change the + /* Since internal frame resolution seems to have changed, we change the * internal frame resolution in use, and call dispmanx_setup_scale() * again to set the rects, etc. */ - _dispvars->width = width; - _dispvars->height = height; + _dispvars->width = width; + _dispvars->height = height; - /* Total pitch, including things the cores + /* Total pitch, including things the cores * render between "visible" scanlines. */ - _dispvars->pitch = pitch; + _dispvars->pitch = pitch; - /* The "visible" width obtained from the core pitch. */ - _dispvars->visible_width = pitch / _dispvars->bytes_per_pixel; + /* The "visible" width obtained from the core pitch. */ + _dispvars->visible_width = pitch / _dispvars->bytes_per_pixel; - dispmanx_free_main_resources(_dispvars); - vc_dispmanx_display_get_info(_dispvars->display, &(_dispvars->amode)); + dispmanx_free_main_resources(_dispvars); + vc_dispmanx_display_get_info(_dispvars->display, &(_dispvars->amode)); - /* We choose the pixel format depending on the bpp of the frame. */ - switch (_dispvars->bytes_per_pixel) + /* We choose the pixel format depending on the bpp of the frame. */ + switch (_dispvars->bytes_per_pixel) { case 2: _dispvars->pixFormat = VC_IMAGE_RGB565; @@ -340,13 +249,13 @@ static bool dispmanx_setup_scale(void *data, unsigned width, return NULL; } - /* Transparency disabled */ - layerAlpha.flags = DISPMANX_FLAGS_ALPHA_FIXED_ALL_PIXELS; - layerAlpha.opacity = 255; - layerAlpha.mask = 0; - _dispvars->alpha = &layerAlpha; + /* Transparency disabled */ + layerAlpha.flags = DISPMANX_FLAGS_ALPHA_FIXED_ALL_PIXELS; + layerAlpha.opacity = 255; + layerAlpha.mask = 0; + _dispvars->alpha = &layerAlpha; - switch (g_settings.video.aspect_ratio_idx) + switch (g_settings.video.aspect_ratio_idx) { case ASPECT_RATIO_4_3: _dispvars->aspect = (float)4 / (float)3; @@ -368,105 +277,107 @@ static bool dispmanx_setup_scale(void *data, unsigned width, break; } - int dst_width = _dispvars->amode.height * _dispvars->aspect; - - /* If we obtain a scaled image width that is bigger than the physical screen width, - * then we keep the physical screen width as our maximun width. */ + int dst_width = _dispvars->amode.height * _dispvars->aspect; + + /* If we obtain a scaled image width that is bigger than the physical screen width, + * then we keep the physical screen width as our maximun width. */ #if 0 - if (dst_width > _dispvars->amode.width) - dst_width = _dispvars->amode.width; + if (dst_width > _dispvars->amode.width) + dst_width = _dispvars->amode.width; #endif - dst_ypos = (_dispvars->amode.width - dst_width) / 2; + dst_ypos = (_dispvars->amode.width - dst_width) / 2; - vc_dispmanx_rect_set(&(_dispvars->dstRect), dst_ypos, 0, - dst_width, _dispvars->amode.height); - - /* We configure the rects now. */ - vc_dispmanx_rect_set(&(_dispvars->bmpRect), 0, 0, _dispvars->width, _dispvars->height); - vc_dispmanx_rect_set(&(_dispvars->srcRect), 0, 0, _dispvars->width << 16, _dispvars->height << 16); + vc_dispmanx_rect_set(&(_dispvars->dstRect), dst_ypos, 0, + dst_width, _dispvars->amode.height); - /* We create as many resources as pages */ - for (i = 0; i < NUMPAGES; i++) - _dispvars->resources[i] = vc_dispmanx_resource_create(_dispvars->pixFormat, - _dispvars->visible_width, _dispvars->height, &(_dispvars->vcImagePtr)); + /* We configure the rects now. */ + vc_dispmanx_rect_set(&(_dispvars->bmpRect), 0, 0, _dispvars->width, _dispvars->height); + vc_dispmanx_rect_set(&(_dispvars->srcRect), 0, 0, _dispvars->width << 16, _dispvars->height << 16); - /* Add element. */ - _dispvars->update = vc_dispmanx_update_start(0); - - _dispvars->element = vc_dispmanx_element_add(_dispvars->update, _dispvars->display, 0, - &(_dispvars->dstRect), _dispvars->resources[0], &(_dispvars->srcRect), - DISPMANX_PROTECTION_NONE, _dispvars->alpha, 0, (DISPMANX_TRANSFORM_T)0); - - vc_dispmanx_update_submit_sync(_dispvars->update); + /* We create as many resources as pages */ + for (i = 0; i < NUMPAGES; i++) + _dispvars->pages[i].resource = vc_dispmanx_resource_create(_dispvars->pixFormat, + _dispvars->visible_width, _dispvars->height, &(_dispvars->vcImagePtr)); - return true; + /* Add element. */ + _dispvars->update = vc_dispmanx_update_start(0); + + _dispvars->element = vc_dispmanx_element_add(_dispvars->update, _dispvars->display, 0, + &(_dispvars->dstRect), _dispvars->pages[0].resource, &(_dispvars->srcRect), + DISPMANX_PROTECTION_NONE, _dispvars->alpha, 0, (DISPMANX_TRANSFORM_T)0); + + vc_dispmanx_update_submit_sync(_dispvars->update); + + return true; } static void dispmanx_update_main(void *data, const void *frame) { - struct dispmanx_page *page = NULL; struct dispmanx_video *_dispvars = data; - if (!_dispvars) - return; - - page = dispmanx_get_free_page(_dispvars); - - if (!page) - return; - + /* Wait until last issued flip completes to get the free page + * that the vsync cb will put in nextPage. Also, dispmanx doesn't + * support issuing more than one pageflip.*/ + slock_lock(_dispvars->pending_mutex); + if (_dispvars->pageflip_pending) + { + scond_wait(_dispvars->vsync_condition, _dispvars->pending_mutex); + } + slock_unlock(_dispvars->pending_mutex); + /* Frame blitting */ - vc_dispmanx_resource_write_data(_dispvars->resources[page->numpage], _dispvars->pixFormat, + vc_dispmanx_resource_write_data(_dispvars->nextPage->resource, _dispvars->pixFormat, _dispvars->pitch, (void *)frame, &(_dispvars->bmpRect)); - /* Issue flipping: we send the page - * to the dispmanx API internal flipping FIFO stack. */ - dispmanx_flip (page, _dispvars); + /* Issue a page flip that will be done at the next vsync. */ + _dispvars->update = vc_dispmanx_update_start(0); + + vc_dispmanx_element_change_source(_dispvars->update, _dispvars->element, + _dispvars->nextPage->resource); + + vc_dispmanx_update_submit(_dispvars->update, vsync_callback, (void*)_dispvars); + + slock_lock(_dispvars->pending_mutex); + _dispvars->pageflip_pending = true; + slock_unlock(_dispvars->pending_mutex); } static void *dispmanx_gfx_init(const video_info_t *video, const input_driver_t **input, void **input_data) { - int i; - struct dispmanx_video *_dispvars = calloc(1, sizeof(struct dispmanx_video)); + int i; + struct dispmanx_video *_dispvars = calloc(1, sizeof(struct dispmanx_video)); if (!_dispvars) return NULL; - _dispvars->bytes_per_pixel = video->rgb32 ? 4 : 2; - _dispvars->screen = 0; - _dispvars->vcImagePtr = 0; - _dispvars->pageflip_pending = 0; - _dispvars->currentPage = NULL; - _dispvars->pages = calloc(NUMPAGES, sizeof(struct dispmanx_page)); - + _dispvars->bytes_per_pixel = video->rgb32 ? 4 : 2; + _dispvars->screen = 0; + _dispvars->vcImagePtr = 0; + _dispvars->pageflip_pending = false; + _dispvars->currentPage = NULL; + _dispvars->pages = calloc(NUMPAGES, sizeof(struct dispmanx_page)); + _dispvars->menu_active = false; + _dispvars->nextPage = &_dispvars->pages[0]; + if (!_dispvars->pages) { free(_dispvars); return NULL; } - for (i = 0; i < NUMPAGES; i++) - { - _dispvars->pages[i].numpage = i; - _dispvars->pages[i].used = false; - _dispvars->pages[i].dispvars = _dispvars; - _dispvars->pages[i].page_used_mutex = slock_new(); - } + /* Initialize the rest of the mutexes and conditions. */ + _dispvars->vsync_condition = scond_new(); + _dispvars->pending_mutex = slock_new(); - /* Initialize the rest of the mutexes and conditions. */ - _dispvars->vsync_condition = scond_new(); - _dispvars->pending_mutex = slock_new(); - _dispvars->vsync_cond_mutex = slock_new(); + bcm_host_init(); - bcm_host_init(); + _dispvars->display = vc_dispmanx_display_open(_dispvars->screen); - _dispvars->display = vc_dispmanx_display_open(_dispvars->screen); + if (input && input_data) + *input = NULL; - if (input && input_data) - *input = NULL; - - return _dispvars; + return _dispvars; } static bool dispmanx_gfx_frame(void *data, const void *frame, unsigned width, @@ -480,31 +391,25 @@ static bool dispmanx_gfx_frame(void *data, const void *frame, unsigned width, if (width != _dispvars->width || height != _dispvars->height) { - /* Sanity check. */ - if (width == 0 || height == 0) - return true; + /* Sanity check. */ + if (width == 0 || height == 0) + return true; - RARCH_LOG("video_dispmanx: internal frame resolution changed by core\n"); - - if (!dispmanx_setup_scale(_dispvars, width, height, pitch)) - { - RARCH_ERR("video_dispmanx: frame resolution set failed\n"); - return false; - } - dispmanx_blank_console (_dispvars); + RARCH_LOG("video_dispmanx: internal frame resolution changed by core\n"); + + if (!dispmanx_setup_scale(_dispvars, width, height, pitch)) + { + RARCH_ERR("video_dispmanx: frame resolution set failed\n"); + return false; + } + + dispmanx_blank_console (_dispvars); } if (_dispvars->menu_active) { char buf[128]; video_monitor_get_fps(buf, sizeof(buf), NULL, 0); - - /* Synchronous flipping of the menu buffers. */ - _dispvars->update = vc_dispmanx_update_start(0); - vc_dispmanx_element_change_source(_dispvars->update, _dispvars->menu_element, - _dispvars->menu_resources[_dispvars->menu_flip_page]); - vc_dispmanx_update_submit_sync(_dispvars->update); - return true; } /* Update main game screen: locate free page, blit and flip. */ @@ -522,9 +427,7 @@ static void dispmanx_free_menu_resources (void *data) _dispvars->update = vc_dispmanx_update_start(0); - vc_dispmanx_resource_delete(_dispvars->menu_resources[0]); - vc_dispmanx_resource_delete(_dispvars->menu_resources[1]); - + vc_dispmanx_resource_delete(_dispvars->menu_resource); vc_dispmanx_element_remove(_dispvars->update, _dispvars->menu_element); vc_dispmanx_update_submit_sync(_dispvars->update); @@ -547,7 +450,7 @@ static void dispmanx_set_texture_frame(void *data, const void *frame, bool rgb32 unsigned width, unsigned height, float alpha) { struct dispmanx_video *_dispvars = data; - + if (!_dispvars) return; @@ -570,7 +473,7 @@ static void dispmanx_set_texture_frame(void *data, const void *frame, bool rgb32 _dispvars->menu_pixFormat = VC_IMAGE_RGBA16; _dispvars->menu_flip_page = 0; - /* Transparency disabled */ + /* Menu transparency has a fixed value for now. */ layerAlpha.flags = DISPMANX_FLAGS_ALPHA_FIXED_ALL_PIXELS; #if 0 layerAlpha.opacity = (unsigned char)(255.0f * alpha); @@ -596,33 +499,26 @@ static void dispmanx_set_texture_frame(void *data, const void *frame, bool rgb32 vc_dispmanx_rect_set(&(_dispvars->menu_bmpRect), 0, 0, _dispvars->menu_width, _dispvars->menu_height); vc_dispmanx_rect_set(&(_dispvars->menu_srcRect), 0, 0, _dispvars->menu_width << 16, _dispvars->menu_height << 16); - /* We create two resources for the menu element. */ - _dispvars->menu_resources[0] = vc_dispmanx_resource_create(_dispvars->menu_pixFormat, - _dispvars->menu_width, _dispvars->menu_height, &(_dispvars->vcImagePtr)); - _dispvars->menu_resources[1] = vc_dispmanx_resource_create(_dispvars->menu_pixFormat, + /* Create resource for the menu element. */ + _dispvars->menu_resource = vc_dispmanx_resource_create(_dispvars->menu_pixFormat, _dispvars->menu_width, _dispvars->menu_height, &(_dispvars->vcImagePtr)); /* Add the menu element. */ _dispvars->update = vc_dispmanx_update_start(0); _dispvars->menu_element = vc_dispmanx_element_add(_dispvars->update, _dispvars->display, 0, - &(_dispvars->menu_dstRect), _dispvars->menu_resources[0], &(_dispvars->menu_srcRect), + &(_dispvars->menu_dstRect), _dispvars->menu_resource, &(_dispvars->menu_srcRect), DISPMANX_PROTECTION_NONE, _dispvars->menu_alpha, 0, (DISPMANX_TRANSFORM_T)0); - vc_dispmanx_update_submit_sync(_dispvars->update); + vc_dispmanx_update_submit_sync(_dispvars->update); } - /* Flipping is done in every frame, - * in the gfx_frame function. - * That's why why change flip page here instead. */ - _dispvars->menu_flip_page = !_dispvars->menu_flip_page; - /* Frame blitting. */ - vc_dispmanx_resource_write_data(_dispvars->menu_resources[_dispvars->menu_flip_page], + vc_dispmanx_resource_write_data(_dispvars->menu_resource, _dispvars->menu_pixFormat, _dispvars->menu_pitch, (void *)frame, &(_dispvars->menu_bmpRect)); - - /* We don't flip the menu buffers here: - * that's done in the gfx_frame function when menu is active. */ + + /* We don't need to flip buffers: one is enough for menu. We sync in gfx_frame, on the + * dispmanx_update_main() call, that is called with menu active, too. */ } static void dispmanx_gfx_set_nonblock_state(void *data, bool state) @@ -637,27 +533,27 @@ static void dispmanx_gfx_set_nonblock_state(void *data, bool state) static bool dispmanx_gfx_alive(void *data) { - (void)data; - return true; /* always alive */ + (void)data; + return true; /* always alive */ } static bool dispmanx_gfx_focus(void *data) { - (void)data; - return true; /* fb device always has focus */ + (void)data; + return true; /* fb device always has focus */ } static void dispmanx_gfx_viewport_info(void *data, struct video_viewport *vp) { - struct dispmanx_video *vid = data; + struct dispmanx_video *vid = data; - if (!vid) - return; + if (!vid) + return; - vp->x = vp->y = 0; + vp->x = vp->y = 0; - vp->width = vp->full_width = vid->width; - vp->height = vp->full_height = vid->height; + vp->width = vp->full_width = vid->width; + vp->height = vp->full_height = vid->height; } static bool dispmanx_gfx_suppress_screensaver(void *data, bool enable) @@ -678,25 +574,25 @@ static bool dispmanx_gfx_has_windowed(void *data) static bool dispmanx_gfx_set_shader(void *data, enum rarch_shader_type type, const char *path) { - (void)data; - (void)type; - (void)path; + (void)data; + (void)type; + (void)path; - return false; + return false; } static void dispmanx_gfx_set_rotation(void *data, unsigned rotation) { - (void)data; - (void)rotation; + (void)data; + (void)rotation; } static bool dispmanx_gfx_read_viewport(void *data, uint8_t *buffer) { - (void)data; - (void)buffer; + (void)data; + (void)buffer; - return true; + return true; } static const video_poke_interface_t dispmanx_poke_interface = { @@ -744,9 +640,6 @@ static void dispmanx_gfx_free(void *data) slock_free(_dispvars->pending_mutex); scond_free(_dispvars->vsync_condition); - for (i = 0; i < NUMPAGES; i++) - slock_free(_dispvars->pages[i].page_used_mutex); - if (_dispvars->pages) free (_dispvars->pages); _dispvars->pages = NULL;