Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 79916028 authored by Rupesh Bansal's avatar Rupesh Bansal
Browse files

Fix delayed offload session start issue 

This fixes the issue where there was a lag between the request to start
the offload session, and when actually it starts. We are fixing this by
acquiring the wakelock when the request comes in, and release it only
when power state transformation is confirmed.

This CL was originally merged and then reverted(ag/28127686) because the
case of DISPLAY.STATE_UNKNOWN was not handled. To start the
watchface, wear transitions the control to AP(For which they override
the doze display state to DISPLAY.STATE_UNKNOWN), the transition occurs
and the display state is again overridden to give the control back to
MCU. This specific workflow was broken, which is now being handled.

Additionally, we were not processing the new request if the old request
to override the dozescreen state was already under process. This
behaviour is now changed to always process the latest requested doze
overridding request.

Bug: 343987330
Flag: com.android.server.display.feature.flags.offload_doze_override_holds_wakelock
Test: Manual
Test: atest DisplayPowerControllerTest
Change-Id: Iab8bce49f26cb25c9c00f0645d15c3c049d2a08d
parent f4c52eab

services/core/java/com/android/server/display/DisplayPowerController.java

+26 −32
Original line number Diff line number Diff line
@@ -492,6 +492,11 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 
    // Used to scale the brightness in doze mode

    private float mDozeScaleFactor;



    // Used to keep track of the display state from the latest request to override the doze screen

    // state.

    @GuardedBy("mLock")

    private int mPendingOverrideDozeScreenStateLocked;



    /**

     * Creates the display power controller.

     */
@@ -803,16 +808,29 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 
    @Override

    public void overrideDozeScreenState(int displayState, @Display.StateReason int reason) {

        Slog.i(TAG, "New offload doze override: " + Display.stateToString(displayState));

        mHandler.postAtTime(() -> {

            if (mDisplayOffloadSession == null

                    || !(DisplayOffloadSession.isSupportedOffloadState(displayState)

        if (mDisplayOffloadSession != null

                && (DisplayOffloadSession.isSupportedOffloadState(displayState)

                || displayState == Display.STATE_UNKNOWN)) {

                return;

            if (mFlags.isOffloadDozeOverrideHoldsWakelockEnabled()) {

                mWakelockController.acquireWakelock(

                        WakelockController.WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE);

            }

            synchronized (mLock) {

                mPendingOverrideDozeScreenStateLocked = displayState;

            }

            mHandler.postAtTime(() -> {

                synchronized (mLock) {

                    mDisplayStateController

                            .overrideDozeScreenState(mPendingOverrideDozeScreenStateLocked, reason);

                }

            mDisplayStateController.overrideDozeScreenState(displayState, reason);

                updatePowerState();

                if (mFlags.isOffloadDozeOverrideHoldsWakelockEnabled()) {

                    mWakelockController.releaseWakelock(

                            WakelockController.WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE);

                }

            }, mClock.uptimeMillis());

        }

    }



    @Override

    public void setDisplayOffloadSession(DisplayOffloadSession session) {
@@ -1338,30 +1356,6 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 
            initialize(readyToUpdateDisplayState() ? state : Display.STATE_UNKNOWN);

        }



        if (mFlags.isOffloadDozeOverrideHoldsWakelockEnabled()) {

            // Sometimes, a display-state change can come without an associated PowerRequest,

            // as with DisplayOffload.  For those cases, we have to make sure to also mark the

            // display as "not ready" so that we can inform power-manager when the state-change is

            // complete.

            if (mPowerState.getScreenState() != state) {

                final boolean wasReady;

                synchronized (mLock) {

                    wasReady = mDisplayReadyLocked;

                    mDisplayReadyLocked = false;

                    mustNotify = true;

                }



                if (wasReady) {

                    // If we went from ready to not-ready from the state-change (instead of a

                    // PowerRequest) there's a good chance that nothing is keeping PowerManager

                    // from suspending. Grab the unfinished business suspend blocker to keep the

                    // device awake until the display-state change goes into effect.

                    mWakelockController.acquireWakelock(

                            WakelockController.WAKE_LOCK_UNFINISHED_BUSINESS);

                }

            }

        }



        // Animate the screen state change unless already animating.

        // The transition may be deferred, so after this point we will use the

        // actual state instead of the desired one.

services/core/java/com/android/server/display/WakelockController.java

+72 −1
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.annotation.IntDef; 
import android.hardware.display.DisplayManagerInternal;

import android.util.Slog;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;

import com.android.server.display.utils.DebugUtils;
@@ -37,7 +38,8 @@ public final class WakelockController { 
    public static final int WAKE_LOCK_PROXIMITY_NEGATIVE = 2;

    public static final int WAKE_LOCK_PROXIMITY_DEBOUNCE = 3;

    public static final int WAKE_LOCK_STATE_CHANGED = 4;

    public static final int WAKE_LOCK_UNFINISHED_BUSINESS = 5;

    public static final int WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE = 5;

    public static final int WAKE_LOCK_UNFINISHED_BUSINESS = 6;



    @VisibleForTesting

    static final int WAKE_LOCK_MAX = WAKE_LOCK_UNFINISHED_BUSINESS;
@@ -53,18 +55,23 @@ public final class WakelockController { 
            WAKE_LOCK_PROXIMITY_NEGATIVE,

            WAKE_LOCK_PROXIMITY_DEBOUNCE,

            WAKE_LOCK_STATE_CHANGED,

            WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE,

            WAKE_LOCK_UNFINISHED_BUSINESS

    })

    @Retention(RetentionPolicy.SOURCE)

    public @interface WAKE_LOCK_TYPE {

    }



    private final Object mLock = new Object();



    // Asynchronous callbacks into the power manager service.

    // Only invoked from the handler thread while no locks are held.

    private final DisplayManagerInternal.DisplayPowerCallbacks mDisplayPowerCallbacks;



    // Identifiers for suspend blocker acquisition requests

    private final String mSuspendBlockerIdUnfinishedBusiness;

    @GuardedBy("mLock")

    private final String mSuspendBlockerOverrideDozeScreenState;

    private final String mSuspendBlockerIdOnStateChanged;

    private final String mSuspendBlockerIdProxPositive;

    private final String mSuspendBlockerIdProxNegative;
@@ -73,6 +80,10 @@ public final class WakelockController { 
    // True if we have unfinished business and are holding a suspend-blocker.

    private boolean mUnfinishedBusiness;



    // True if we have are holding a suspend-blocker to override the doze screen state.

    @GuardedBy("mLock")

    private boolean mIsOverrideDozeScreenStateAcquired;



    // True if we have have debounced the proximity change impact and are holding a suspend-blocker.

    private boolean mHasProximityDebounced;
@@ -108,6 +119,7 @@ public final class WakelockController { 
        mTag = TAG + "[" + mDisplayId + "]";

        mDisplayPowerCallbacks = callbacks;

        mSuspendBlockerIdUnfinishedBusiness = "[" + displayId + "]unfinished business";

        mSuspendBlockerOverrideDozeScreenState =  "[" + displayId + "]override doze screen state";

        mSuspendBlockerIdOnStateChanged = "[" + displayId + "]on state changed";

        mSuspendBlockerIdProxPositive = "[" + displayId + "]prox positive";

        mSuspendBlockerIdProxNegative = "[" + displayId + "]prox negative";
@@ -154,6 +166,10 @@ public final class WakelockController { 
                return acquireProxDebounceSuspendBlocker();

            case WAKE_LOCK_STATE_CHANGED:

                return acquireStateChangedSuspendBlocker();

            case WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE:

                synchronized (mLock) {

                    return acquireOverrideDozeScreenStateSuspendBlockerLocked();

                }

            case WAKE_LOCK_UNFINISHED_BUSINESS:

                return acquireUnfinishedBusinessSuspendBlocker();

            default:
@@ -171,6 +187,10 @@ public final class WakelockController { 
                return releaseProxDebounceSuspendBlocker();

            case WAKE_LOCK_STATE_CHANGED:

                return releaseStateChangedSuspendBlocker();

            case WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE:

                synchronized (mLock) {

                    return releaseOverrideDozeScreenStateSuspendBlockerLocked();

                }

            case WAKE_LOCK_UNFINISHED_BUSINESS:

                return releaseUnfinishedBusinessSuspendBlocker();

            default:
@@ -219,6 +239,42 @@ public final class WakelockController { 
        return false;

    }



    /**

     * Acquires the suspend blocker to override the doze screen state and notifies the

     * PowerManagerService about the changes. Note that this utility is syncronized because a

     * request to override the doze screen state can come from a non-power thread.

     */

    @GuardedBy("mLock")

    private boolean acquireOverrideDozeScreenStateSuspendBlockerLocked() {

        // Grab a wake lock if we have unfinished business.

        if (!mIsOverrideDozeScreenStateAcquired) {

            if (DEBUG) {

                Slog.d(mTag, "Acquiring suspend blocker to override the doze screen state...");

            }

            mDisplayPowerCallbacks.acquireSuspendBlocker(mSuspendBlockerOverrideDozeScreenState);

            mIsOverrideDozeScreenStateAcquired = true;

            return true;

        }

        return false;

    }



    /**

     * Releases the override doze screen state suspend blocker and notifies the PowerManagerService

     * about the changes.

     */

    @GuardedBy("mLock")

    private boolean releaseOverrideDozeScreenStateSuspendBlockerLocked() {

        if (mIsOverrideDozeScreenStateAcquired) {

            if (DEBUG) {

                Slog.d(mTag, "Finished overriding doze screen state...");

            }

            mDisplayPowerCallbacks.releaseSuspendBlocker(mSuspendBlockerOverrideDozeScreenState);

            mIsOverrideDozeScreenStateAcquired = false;

            return true;

        }

        return false;

    }



    /**

     * Acquires the unfinished business wakelock and notifies the PowerManagerService about the

     * changes.
@@ -366,6 +422,7 @@ public final class WakelockController { 
        pw.println("  mOnStateChangePending=" + isOnStateChangedPending());

        pw.println("  mOnProximityPositiveMessages=" + isProximityPositiveAcquired());

        pw.println("  mOnProximityNegativeMessages=" + isProximityNegativeAcquired());

        pw.println("  mIsOverrideDozeScreenStateAcquired=" + isOverrideDozeScreenStateAcquired());

    }



    @VisibleForTesting
@@ -393,6 +450,13 @@ public final class WakelockController { 
        return mSuspendBlockerIdProxDebounce;

    }



    @VisibleForTesting

    String getSuspendBlockerOverrideDozeScreenState() {

        synchronized (mLock) {

            return mSuspendBlockerOverrideDozeScreenState;

        }

    }



    @VisibleForTesting

    boolean hasUnfinishedBusiness() {

        return mUnfinishedBusiness;
@@ -417,4 +481,11 @@ public final class WakelockController { 
    boolean hasProximitySensorDebounced() {

        return mHasProximityDebounced;

    }



    @VisibleForTesting

    boolean isOverrideDozeScreenStateAcquired() {

        synchronized (mLock) {

            return mIsOverrideDozeScreenStateAcquired;

        }

    }

}

services/tests/displayservicetests/src/com/android/server/display/DisplayPowerControllerTest.java

+8 −3
Original line number Diff line number Diff line
@@ -1621,16 +1621,21 @@ public final class DisplayPowerControllerTest { 
        advanceTime(1); // Run updatePowerState



        reset(mHolder.wakelockController);

        when(mHolder.wakelockController

                .acquireWakelock(WakelockController.WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE))

                .thenReturn(true);

        mHolder.dpc.overrideDozeScreenState(

                supportedTargetState, Display.STATE_REASON_DEFAULT_POLICY);

        advanceTime(1); // Run updatePowerState



        // Should get a wakelock to notify powermanager

        verify(mHolder.wakelockController, atLeastOnce()).acquireWakelock(

                eq(WakelockController.WAKE_LOCK_UNFINISHED_BUSINESS));

        verify(mHolder.wakelockController).acquireWakelock(

                eq(WakelockController.WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE));



        advanceTime(1); // Run updatePowerState

        verify(mHolder.displayPowerState)

                .setScreenState(supportedTargetState, Display.STATE_REASON_DEFAULT_POLICY);

        verify(mHolder.wakelockController).releaseWakelock(

                eq(WakelockController.WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE));

    }



    @Test

services/tests/displayservicetests/src/com/android/server/display/WakelockControllerTest.java

+24 −0
Original line number Diff line number Diff line
@@ -64,6 +64,8 @@ public final class WakelockControllerTest { 
                "[" + DISPLAY_ID + "]prox negative");

        assertEquals(mWakelockController.getSuspendBlockerProxDebounceId(),

                "[" + DISPLAY_ID + "]prox debounce");

        assertEquals(mWakelockController.getSuspendBlockerOverrideDozeScreenState(),

                "[" + DISPLAY_ID + "]override doze screen state");

    }



    @Test
@@ -161,6 +163,28 @@ public final class WakelockControllerTest { 
                .releaseSuspendBlocker(mWakelockController.getSuspendBlockerProxDebounceId());

    }



    @Test

    public void acquireOverrideDozeScreenStateSuspendBlocker() throws Exception {

        // Acquire the suspend blocker

        verifyWakelockAcquisitionAndReaquisition(WakelockController

                        .WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE,

                () -> mWakelockController.isOverrideDozeScreenStateAcquired());



        // Verify acquire happened only once

        verify(mDisplayPowerCallbacks, times(1))

                .acquireSuspendBlocker(mWakelockController

                        .getSuspendBlockerOverrideDozeScreenState());



        // Release the suspend blocker

        verifyWakelockReleaseAndRerelease(WakelockController.WAKE_LOCK_OVERRIDE_DOZE_SCREEN_STATE,

                () -> mWakelockController.isOverrideDozeScreenStateAcquired());



        // Verify suspend blocker was released only once

        verify(mDisplayPowerCallbacks, times(1))

                .releaseSuspendBlocker(mWakelockController

                        .getSuspendBlockerOverrideDozeScreenState());

    }



    @Test

    public void proximityPositiveRunnableWorksAsExpected() {

        // Acquire the suspend blocker twice