Skip to content

Commit

Permalink
hwc: fixes for vsync and race conditions
Browse files Browse the repository at this point in the history 
- Fix some race conditions between hwc_eventControl and the hwcVsyncThread.
- don't call vsync-ctrl ioctl for same enable value
- move enable disable ioctls to vsync thread, this stabilizes the vsync
  signal
- read again if vsync read is interrupted

Change-Id: I4065918b352769dd9fe18bd0dff4a87053299189
CRs-Fixed: 393243
CRs-Fixed: 416487
  • Loading branch information
@arco
Arun Kumar K.R authored and arco committed Mar 23, 2013
1 parent 877f58e commit d913a95
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 88 deletions.
46 changes: 11 additions & 35 deletions libhwcomposer/hwc.cpp
View file
Expand Up @@ -143,48 +143,24 @@ static int hwc_eventControl(struct hwc_composer_device* dev,
int event, int value)
{
int ret = 0;
static int prev_value, temp;

hwc_context_t* ctx = (hwc_context_t*)(dev);
private_module_t* m = reinterpret_cast<private_module_t*>(
ctx->mFbDev->common.module);
switch(event) {
#ifndef NO_HW_VSYNC
case HWC_EVENT_VSYNC:
if (value == prev_value){
//TODO see why HWC gets repeated events
ALOGD_IF(VSYNC_DEBUG, "%s - VSYNC is already %s",
__FUNCTION__, (value)?"ENABLED":"DISABLED");
}
temp = ctx->vstate.enable;
if(ioctl(m->framebuffer->fd, MSMFB_OVERLAY_VSYNC_CTRL, &value) < 0)
ret = -errno;

/* vsync state change logic */
if (value == 1) {
//unblock vsync thread
pthread_mutex_lock(&ctx->vstate.lock);
ctx->vstate.enable = true;
pthread_cond_signal(&ctx->vstate.cond);
pthread_mutex_unlock(&ctx->vstate.lock);
}
if (value == 0 && temp) {
//vsync thread will block
pthread_mutex_lock(&ctx->vstate.lock);
ctx->vstate.enable = false;
pthread_mutex_unlock(&ctx->vstate.lock);
}
ALOGD_IF (VSYNC_DEBUG, "VSYNC state changed from %s to %s",
(prev_value)?"ENABLED":"DISABLED", (value)?"ENABLED":"DISABLED");
prev_value = value;
/* vsync state change logic - end*/

if(ctx->mExtDisplay->isHDMIConfigured() &&
(ctx->mExtDisplay->getExternalDisplay()==EXTERN_DISPLAY_FB1)) {
// enableHDMIVsync will return -errno on error
ret = ctx->mExtDisplay->enableHDMIVsync(value);
}
break;
if (ctx->vstate.enable == value)
break;

pthread_mutex_lock(&ctx->vstate.lock);
ctx->vstate.enable = !!value;
pthread_cond_signal(&ctx->vstate.cond);

ALOGD_IF (VSYNC_DEBUG, "VSYNC state changed to %s",
(value)?"ENABLED":"DISABLED");
pthread_mutex_unlock(&ctx->vstate.lock);
break;
#endif
case HWC_EVENT_ORIENTATION:
ctx->deviceOrientation = value;
Expand Down
153 changes: 100 additions & 53 deletions libhwcomposer/hwc_vsync.cpp
View file
Expand Up @@ -25,6 +25,10 @@
#include <fcntl.h>
#include <sys/resource.h>
#include <sys/prctl.h>
#include <sys/ioctl.h>
#include <gralloc_priv.h>
#include <fb_priv.h>
#include <linux/msm_mdp.h>
#include "hwc_utils.h"
#include "hwc_external.h"
#include "string.h"
Expand All @@ -39,17 +43,23 @@ static void *vsync_loop(void *param)
const char* vsync_timestamp_fb1 = "/sys/class/graphics/fb1/vsync_event";

hwc_context_t * ctx = reinterpret_cast<hwc_context_t *>(param);

private_module_t* m = reinterpret_cast<private_module_t*>(
ctx->mFbDev->common.module);
char thread_name[64] = "hwcVsyncThread";
prctl(PR_SET_NAME, (unsigned long) &thread_name, 0, 0, 0);
setpriority(PRIO_PROCESS, 0,
HAL_PRIORITY_URGENT_DISPLAY + ANDROID_PRIORITY_MORE_FAVORABLE);

static char vdata[PAGE_SIZE];
const int MAX_DATA = 64;
const int MAX_RETRY_COUNT = 100;
static char vdata[MAX_DATA];

uint64_t cur_timestamp=0;
int32_t len = -1, fd_timestamp = -1;
ssize_t len = -1;
int fd_timestamp = -1;
int ret = 0;
bool fb1_vsync = false;
bool enabled = false;

/* Currently read vsync timestamp from drivers
e.g. VSYNC=41800875994
Expand All @@ -58,65 +68,102 @@ static void *vsync_loop(void *param)
hwc_procs* proc = (hwc_procs*)ctx->device.reserved_proc[0];

do {
int hdmiconnected = ctx->mExtDisplay->getExternalDisplay();

// vsync for primary OR HDMI ?
if(ctx->mExtDisplay->isHDMIConfigured() &&
(hdmiconnected == EXTERN_DISPLAY_FB1)){
fb1_vsync = true;
} else {
fb1_vsync = false;
}

pthread_mutex_lock(&ctx->vstate.lock);
if(ctx->vstate.enable == false) {
pthread_cond_wait(&ctx->vstate.cond, &ctx->vstate.lock);
while (ctx->vstate.enable == false) {
if(enabled) {
int e = 0;
if(ioctl(m->framebuffer->fd, MSMFB_OVERLAY_VSYNC_CTRL,
&e) < 0) {
ALOGE("%s: vsync control failed for fb0 enabled=%d : %s",
__FUNCTION__, enabled, strerror(errno));
ret = -errno;
}
if(fb1_vsync) {
ret = ctx->mExtDisplay->enableHDMIVsync(e);
}
enabled = false;
}
pthread_cond_wait(&ctx->vstate.cond, &ctx->vstate.lock);
}
pthread_mutex_unlock(&ctx->vstate.lock);

int hdmiconnected = ctx->mExtDisplay->getExternalDisplay();
if (!enabled) {
int e = 1;
if(ioctl(m->framebuffer->fd, MSMFB_OVERLAY_VSYNC_CTRL,
&e) < 0) {
ALOGE("%s: vsync control failed for fb0 enabled=%d : %s",
__FUNCTION__, enabled, strerror(errno));
ret = -errno;
}
if(fb1_vsync) {
ret = ctx->mExtDisplay->enableHDMIVsync(e);
}
enabled = true;
}

// vsync for primary OR HDMI ?
if(ctx->mExtDisplay->isHDMIConfigured() &&
(hdmiconnected == EXTERN_DISPLAY_FB1)){
fb1_vsync = true;
} else {
fb1_vsync = false;
}

// try to open timestamp sysfs
if (fb1_vsync){
fd_timestamp = open(vsync_timestamp_fb1, O_RDONLY);
} else {
fd_timestamp = open(vsync_timestamp_fb0, O_RDONLY);
}
if (fd_timestamp < 0) {
ALOGE ("FATAL:%s:not able to open file:%s, %s", __FUNCTION__,
(fb1_vsync) ? vsync_timestamp_fb1 : vsync_timestamp_fb0,
// try to open timestamp sysfs
if (fb1_vsync){
fd_timestamp = open(vsync_timestamp_fb1, O_RDONLY);
} else {
fd_timestamp = open(vsync_timestamp_fb0, O_RDONLY);
}
if (fd_timestamp < 0) {
ALOGE ("FATAL:%s:not able to open file:%s, %s", __FUNCTION__,
(fb1_vsync) ? vsync_timestamp_fb1 : vsync_timestamp_fb0,
strerror(errno));
return NULL;
}
// Open success - read now
len = read(fd_timestamp, vdata, PAGE_SIZE);
if (len < 0){
ALOGE ("FATAL:%s:not able to read file:%s, %s", __FUNCTION__,
(fb1_vsync) ? vsync_timestamp_fb1 : vsync_timestamp_fb0,
strerror(errno));
fd_timestamp = -1;
return NULL;
}

// extract timestamp
const char *str = vdata;
if (!strncmp(str, "VSYNC=", strlen("VSYNC="))) {
cur_timestamp = strtoull(str + strlen("VSYNC="), NULL, 0);
} else {
ALOGE ("FATAL:%s:timestamp data not in correct format",
__FUNCTION__);
}
// send timestamp to HAL
ALOGD_IF (VSYNC_DEBUG, "%s: timestamp %llu sent to HWC for %s",
__FUNCTION__, cur_timestamp, (fb1_vsync) ? "fb1" : "fb0");
proc->vsync(proc, 0, cur_timestamp);

// close open fds
close (fd_timestamp);

// reset fd
fd_timestamp = -1;
return NULL;
}
for(int i = 0; i < MAX_RETRY_COUNT; i++) {
len = pread(fd_timestamp, vdata, MAX_DATA, 0);
if(len < 0 && errno == EAGAIN) {
ALOGW("%s: vsync read: EAGAIN, retry (%d/%d).",
__FUNCTION__, i, MAX_RETRY_COUNT);
continue;
} else {
break;
}
}

if (len < 0){
ALOGE ("FATAL:%s:not able to read file:%s, %s", __FUNCTION__,
(fb1_vsync) ? vsync_timestamp_fb1 : vsync_timestamp_fb0,
strerror(errno));
close (fd_timestamp);
fd_timestamp = -1;
return NULL;
}

// extract timestamp
const char *str = vdata;
if (!strncmp(str, "VSYNC=", strlen("VSYNC="))) {
cur_timestamp = strtoull(str + strlen("VSYNC="), NULL, 0);
} else {
ALOGE ("FATAL:%s:timestamp data not in correct format",
__FUNCTION__);
}
// send timestamp to HAL
ALOGD_IF (VSYNC_DEBUG, "%s: timestamp %llu sent to HWC for %s",
__FUNCTION__, cur_timestamp, (fb1_vsync) ? "fb1" : "fb0");
proc->vsync(proc, 0, cur_timestamp);
// close open fds
if(fd_timestamp >= 0)
close (fd_timestamp);
// reset fd
fd_timestamp = -1;
// repeat, whatever, you just did
} while (true);

return NULL;
}

void init_vsync_thread(hwc_context_t* ctx)
Expand Down

0 comments on commit d913a95

Please sign in to comment.