hwc: Fix locking.

Remove the unnecessary blank lock, mdp comp lock, secure lock.
Rename the ext lock to a more appropriate draw lock.

The mdp comp lock is at an incorrect place and causes unwanted
objects to show up in dumpsys, since configDone hasnt cleaned
them up yet.

dump(), blank(), draw() should all acquire a common lock.
draw() includes prepare() and set().

Change-Id: I595547dd5a393a8af6cd8c9297d50793b715e658
This commit is contained in:
Saurabh Shah
2013-08-22 10:21:44 -07:00
parent c2f58ae4b3
commit b39f8151ab
6 changed files with 97 additions and 119 deletions

View File

@@ -257,10 +257,8 @@ static int hwc_prepare(hwc_composer_device_1 *dev, size_t numDisplays,
{ {
int ret = 0; int ret = 0;
hwc_context_t* ctx = (hwc_context_t*)(dev); hwc_context_t* ctx = (hwc_context_t*)(dev);
ctx->mBlankLock.lock();
//Will be unlocked at the end of set //Will be unlocked at the end of set
ctx->mExtLock.lock(); ctx->mDrawLock.lock();
ctx->mSecureLock.lock();
reset(ctx, numDisplays, displays); reset(ctx, numDisplays, displays);
ctx->mOverlay->configBegin(); ctx->mOverlay->configBegin();
@@ -312,7 +310,7 @@ static int hwc_eventControl(struct hwc_composer_device_1* dev, int dpy,
#ifdef QCOM_BSP #ifdef QCOM_BSP
case HWC_EVENT_ORIENTATION: case HWC_EVENT_ORIENTATION:
if(dpy == HWC_DISPLAY_PRIMARY) { if(dpy == HWC_DISPLAY_PRIMARY) {
Locker::Autolock _l(ctx->mBlankLock); Locker::Autolock _l(ctx->mDrawLock);
// store the primary display orientation // store the primary display orientation
// will be used in hwc_video::configure to disable // will be used in hwc_video::configure to disable
// rotation animation on external display // rotation animation on external display
@@ -331,7 +329,7 @@ static int hwc_blank(struct hwc_composer_device_1* dev, int dpy, int blank)
ATRACE_CALL(); ATRACE_CALL();
hwc_context_t* ctx = (hwc_context_t*)(dev); hwc_context_t* ctx = (hwc_context_t*)(dev);
Locker::Autolock _l(ctx->mBlankLock); Locker::Autolock _l(ctx->mDrawLock);
int ret = 0, value = 0; int ret = 0, value = 0;
ALOGD_IF(BLANK_DEBUG, "%s: %s display: %d", __FUNCTION__, ALOGD_IF(BLANK_DEBUG, "%s: %s display: %d", __FUNCTION__,
blank==1 ? "Blanking":"Unblanking", dpy); blank==1 ? "Blanking":"Unblanking", dpy);
@@ -615,9 +613,7 @@ static int hwc_set(hwc_composer_device_1 *dev,
MDPComp::resetIdleFallBack(); MDPComp::resetIdleFallBack();
ctx->mVideoTransFlag = false; ctx->mVideoTransFlag = false;
//Was locked at the beginning of prepare //Was locked at the beginning of prepare
ctx->mSecureLock.unlock(); ctx->mDrawLock.unlock();
ctx->mExtLock.unlock();
ctx->mBlankLock.unlock();
return ret; return ret;
} }
@@ -707,6 +703,7 @@ int hwc_getDisplayAttributes(struct hwc_composer_device_1* dev, int disp,
void hwc_dump(struct hwc_composer_device_1* dev, char *buff, int buff_len) void hwc_dump(struct hwc_composer_device_1* dev, char *buff, int buff_len)
{ {
hwc_context_t* ctx = (hwc_context_t*)(dev); hwc_context_t* ctx = (hwc_context_t*)(dev);
Locker::Autolock _l(ctx->mDrawLock);
android::String8 aBuf(""); android::String8 aBuf("");
dumpsys_log(aBuf, "Qualcomm HWC state:\n"); dumpsys_log(aBuf, "Qualcomm HWC state:\n");
dumpsys_log(aBuf, " MDPVersion=%d\n", ctx->mMDP.version); dumpsys_log(aBuf, " MDPVersion=%d\n", ctx->mMDP.version);

View File

@@ -55,7 +55,6 @@ MDPComp::MDPComp(int dpy):mDpy(dpy){};
void MDPComp::dump(android::String8& buf) void MDPComp::dump(android::String8& buf)
{ {
Locker::Autolock _l(mMdpCompLock);
dumpsys_log(buf,"HWC Map for Dpy: %s \n", dumpsys_log(buf,"HWC Map for Dpy: %s \n",
(mDpy == 0) ? "\"PRIMARY\"" : (mDpy == 0) ? "\"PRIMARY\"" :
(mDpy == 1) ? "\"EXTERNAL\"" : "\"VIRTUAL\""); (mDpy == 1) ? "\"EXTERNAL\"" : "\"VIRTUAL\"");
@@ -738,9 +737,6 @@ bool MDPComp::programYUV(hwc_context_t *ctx, hwc_display_contents_1_t* list) {
int MDPComp::prepare(hwc_context_t *ctx, hwc_display_contents_1_t* list) { int MDPComp::prepare(hwc_context_t *ctx, hwc_display_contents_1_t* list) {
const int numLayers = ctx->listStats[mDpy].numAppLayers; const int numLayers = ctx->listStats[mDpy].numAppLayers;
{ //LOCK SCOPE BEGIN
Locker::Autolock _l(mMdpCompLock);
//reset old data //reset old data
mCurrentFrame.reset(numLayers); mCurrentFrame.reset(numLayers);
@@ -762,7 +758,7 @@ int MDPComp::prepare(hwc_context_t *ctx, hwc_display_contents_1_t* list) {
} }
//Check whether layers marked for MDP Composition is actually doable. //Check whether layers marked for MDP Composition is actually doable.
if(isFullFrameDoable(ctx, list)){ if(isFullFrameDoable(ctx, list)) {
mCurrentFrame.map(); mCurrentFrame.map();
//Configure framebuffer first if applicable //Configure framebuffer first if applicable
if(mCurrentFrame.fbZ >= 0) { if(mCurrentFrame.fbZ >= 0) {
@@ -823,7 +819,6 @@ int MDPComp::prepare(hwc_context_t *ctx, hwc_display_contents_1_t* list) {
setMDPCompLayerFlags(ctx, list); setMDPCompLayerFlags(ctx, list);
mCachedFrame.updateCounts(mCurrentFrame); mCachedFrame.updateCounts(mCurrentFrame);
} //LOCK SCOPE END. dump also need this lock.
// unlock it before calling dump function to avoid deadlock // unlock it before calling dump function to avoid deadlock
if(isDebug()) { if(isDebug()) {
ALOGD("GEOMETRY change: %d", (list->flags & HWC_GEOMETRY_CHANGED)); ALOGD("GEOMETRY change: %d", (list->flags & HWC_GEOMETRY_CHANGED));
@@ -925,8 +920,6 @@ bool MDPCompLowRes::draw(hwc_context_t *ctx, hwc_display_contents_1_t* list) {
return true; return true;
} }
Locker::Autolock _l(mMdpCompLock);
/* reset Invalidator */ /* reset Invalidator */
if(idleInvalidator && !sIdleFallBack && mCurrentFrame.mdpCount) if(idleInvalidator && !sIdleFallBack && mCurrentFrame.mdpCount)
idleInvalidator->markForSleep(); idleInvalidator->markForSleep();
@@ -1132,8 +1125,6 @@ bool MDPCompHighRes::draw(hwc_context_t *ctx, hwc_display_contents_1_t* list) {
return true; return true;
} }
Locker::Autolock _l(mMdpCompLock);
/* reset Invalidator */ /* reset Invalidator */
if(idleInvalidator && !sIdleFallBack && mCurrentFrame.mdpCount) if(idleInvalidator && !sIdleFallBack && mCurrentFrame.mdpCount)
idleInvalidator->markForSleep(); idleInvalidator->markForSleep();

View File

@@ -170,7 +170,6 @@ protected:
static IdleInvalidator *idleInvalidator; static IdleInvalidator *idleInvalidator;
struct FrameInfo mCurrentFrame; struct FrameInfo mCurrentFrame;
struct LayerCache mCachedFrame; struct LayerCache mCachedFrame;
mutable Locker mMdpCompLock;
}; };
class MDPCompLowRes : public MDPComp { class MDPCompLowRes : public MDPComp {

View File

@@ -74,7 +74,7 @@ status_t QClient::notifyCallback(uint32_t msg, uint32_t value) {
} }
void QClient::securing(uint32_t startEnd) { void QClient::securing(uint32_t startEnd) {
Locker::Autolock _sl(mHwcContext->mSecureLock); Locker::Autolock _sl(mHwcContext->mDrawLock);
//The only way to make this class in this process subscribe to media //The only way to make this class in this process subscribe to media
//player's death. //player's death.
IMediaDeathNotifier::getMediaPlayerService(); IMediaDeathNotifier::getMediaPlayerService();
@@ -88,7 +88,7 @@ void QClient::securing(uint32_t startEnd) {
} }
void QClient::unsecuring(uint32_t startEnd) { void QClient::unsecuring(uint32_t startEnd) {
Locker::Autolock _sl(mHwcContext->mSecureLock); Locker::Autolock _sl(mHwcContext->mDrawLock);
mHwcContext->mSecuring = startEnd; mHwcContext->mSecuring = startEnd;
//We're done unsecuring //We're done unsecuring
if(startEnd == IQService::END) if(startEnd == IQService::END)
@@ -98,7 +98,7 @@ void QClient::unsecuring(uint32_t startEnd) {
} }
void QClient::MPDeathNotifier::died() { void QClient::MPDeathNotifier::died() {
Locker::Autolock _sl(mHwcContext->mSecureLock); Locker::Autolock _sl(mHwcContext->mDrawLock);
ALOGD_IF(QCLIENT_DEBUG, "Media Player died"); ALOGD_IF(QCLIENT_DEBUG, "Media Player died");
mHwcContext->mSecuring = false; mHwcContext->mSecuring = false;
mHwcContext->mSecureMode = false; mHwcContext->mSecureMode = false;

View File

@@ -125,7 +125,7 @@ static void handle_uevent(hwc_context_t* ctx, const char* udata, int len)
break; break;
} }
Locker::Autolock _l(ctx->mExtLock); Locker::Autolock _l(ctx->mDrawLock);
clear(ctx, dpy); clear(ctx, dpy);
ctx->dpyAttr[dpy].connected = false; ctx->dpyAttr[dpy].connected = false;
ctx->dpyAttr[dpy].isActive = false; ctx->dpyAttr[dpy].isActive = false;
@@ -164,7 +164,7 @@ static void handle_uevent(hwc_context_t* ctx, const char* udata, int len)
//fail, since Layer Mixer#0 is still connected to WriteBack. //fail, since Layer Mixer#0 is still connected to WriteBack.
//This block will force composition to close fb2 in above //This block will force composition to close fb2 in above
//example. //example.
Locker::Autolock _l(ctx->mExtLock); Locker::Autolock _l(ctx->mDrawLock);
ctx->dpyAttr[dpy].isConfiguring = true; ctx->dpyAttr[dpy].isConfiguring = true;
ctx->proc->invalidate(ctx->proc); ctx->proc->invalidate(ctx->proc);
} }
@@ -177,7 +177,7 @@ static void handle_uevent(hwc_context_t* ctx, const char* udata, int len)
ALOGD_IF(UEVENT_DEBUG,"Received HDMI connection request" ALOGD_IF(UEVENT_DEBUG,"Received HDMI connection request"
"when WFD is active"); "when WFD is active");
{ {
Locker::Autolock _l(ctx->mExtLock); Locker::Autolock _l(ctx->mDrawLock);
clear(ctx, HWC_DISPLAY_VIRTUAL); clear(ctx, HWC_DISPLAY_VIRTUAL);
ctx->dpyAttr[HWC_DISPLAY_VIRTUAL].connected = false; ctx->dpyAttr[HWC_DISPLAY_VIRTUAL].connected = false;
ctx->dpyAttr[HWC_DISPLAY_VIRTUAL].isActive = false; ctx->dpyAttr[HWC_DISPLAY_VIRTUAL].isActive = false;
@@ -193,7 +193,7 @@ static void handle_uevent(hwc_context_t* ctx, const char* udata, int len)
ctx->proc->hotplug(ctx->proc, HWC_DISPLAY_EXTERNAL, ctx->proc->hotplug(ctx->proc, HWC_DISPLAY_EXTERNAL,
EXTERNAL_OFFLINE); EXTERNAL_OFFLINE);
{ {
Locker::Autolock _l(ctx->mExtLock); Locker::Autolock _l(ctx->mDrawLock);
ctx->mVirtualonExtActive = false; ctx->mVirtualonExtActive = false;
} }
} }
@@ -205,7 +205,7 @@ static void handle_uevent(hwc_context_t* ctx, const char* udata, int len)
ctx->mExtDisplay->configure(); ctx->mExtDisplay->configure();
} else { } else {
{ {
Locker::Autolock _l(ctx->mExtLock); Locker::Autolock _l(ctx->mDrawLock);
/* TRUE only when we are on proprietary WFD session */ /* TRUE only when we are on proprietary WFD session */
ctx->mVirtualonExtActive = true; ctx->mVirtualonExtActive = true;
char property[PROPERTY_VALUE_MAX]; char property[PROPERTY_VALUE_MAX];
@@ -220,7 +220,7 @@ static void handle_uevent(hwc_context_t* ctx, const char* udata, int len)
ctx->mVirtualDisplay->configure(); ctx->mVirtualDisplay->configure();
} }
Locker::Autolock _l(ctx->mExtLock); Locker::Autolock _l(ctx->mDrawLock);
setup(ctx, dpy); setup(ctx, dpy);
ctx->dpyAttr[dpy].isPause = false; ctx->dpyAttr[dpy].isPause = false;
ctx->dpyAttr[dpy].connected = true; ctx->dpyAttr[dpy].connected = true;
@@ -255,7 +255,7 @@ static void handle_uevent(hwc_context_t* ctx, const char* udata, int len)
//Since external didnt have any pipes, force primary to give up //Since external didnt have any pipes, force primary to give up
//its pipes; we don't allow inter-mixer pipe transfers. //its pipes; we don't allow inter-mixer pipe transfers.
{ {
Locker::Autolock _l(ctx->mExtLock); Locker::Autolock _l(ctx->mDrawLock);
ctx->dpyAttr[dpy].isConfiguring = true; ctx->dpyAttr[dpy].isConfiguring = true;
ctx->dpyAttr[dpy].isActive = true; ctx->dpyAttr[dpy].isActive = true;
ctx->proc->invalidate(ctx->proc); ctx->proc->invalidate(ctx->proc);
@@ -264,7 +264,7 @@ static void handle_uevent(hwc_context_t* ctx, const char* udata, int len)
* 2 / 1000); * 2 / 1000);
{ {
//At this point external has all the pipes it would need. //At this point external has all the pipes it would need.
Locker::Autolock _l(ctx->mExtLock); Locker::Autolock _l(ctx->mDrawLock);
ctx->dpyAttr[dpy].isPause = false; ctx->dpyAttr[dpy].isPause = false;
ctx->proc->invalidate(ctx->proc); ctx->proc->invalidate(ctx->proc);
} }

View File

@@ -357,17 +357,8 @@ struct hwc_context_t {
bool mVirtualonExtActive; bool mVirtualonExtActive;
//Display in secure mode indicator //Display in secure mode indicator
bool mSecureMode; bool mSecureMode;
//Lock to prevent set from being called while blanking //Lock to protect drawing data structures
mutable Locker mBlankLock; mutable Locker mDrawLock;
//Lock to protect prepare & set when detaching external disp
mutable Locker mExtLock;
/*Lock to set both mSecureMode and mSecuring as part
of binder thread without context switch to composition
thread. This lock is needed only for A-family targets
since the state of mSecureMode and mSecuring variables
are not checked in B-family targets.
*/
mutable Locker mSecureLock;
//Drawing round when we use GPU //Drawing round when we use GPU
bool isPaddingRound; bool isPaddingRound;
// External Orientation // External Orientation