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

Commit a8c72597 authored by Santos Cordon's avatar Santos Cordon Committed by Rupesh Bansal
Browse files

Offload Doze Override should trigger suspend blocker 

Unlike most display-state-changes, Offload state changes are not
triggered by a PowerRequest.  Because of this PowerManager isn't aware
of the change and doesn't attempt to keep the device awake.  As a
result, offload transitions sometimes went into suspend before they were
complete, preventing offload from running when it should.

This change adds a suspend blocker to offload doze override requests,
and notifies PowerManager when it's processing a change.

Test: atest com.android.server.display
Bug: 336599466
Change-Id: Ib9a6eeeeb3bc0c2c66ac02dff1aec61203e6b04c
parent f60aa416

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

+17 −1
Original line number Diff line number Diff line
@@ -21,13 +21,21 @@ import static com.android.server.display.AutomaticBrightnessController.AUTO_BRIG 
import android.annotation.Nullable;

import android.hardware.display.DisplayManagerInternal;

import android.os.Trace;

import android.util.Slog;

import android.view.Display;



import com.android.server.display.utils.DebugUtils;



/**

 * An implementation of the offload session that keeps track of whether the session is active.

 * An offload session is used to control the display's brightness using the offload chip.

 */

public class DisplayOffloadSessionImpl implements DisplayManagerInternal.DisplayOffloadSession {

    private static final String TAG = "DisplayOffloadSessionImpl";



    // To enable these logs, run:

    // 'adb shell setprop persist.log.tag.DisplayOffloadSessionImpl DEBUG && adb reboot'

    private static final boolean DEBUG = DebugUtils.isDebuggable(TAG);



    @Nullable

    private final DisplayManagerInternal.DisplayOffloader mDisplayOffloader;
@@ -91,9 +99,14 @@ public class DisplayOffloadSessionImpl implements DisplayManagerInternal.Display 
        if (mDisplayOffloader == null || mIsActive) {

            return false;

        }



        Trace.traceBegin(Trace.TRACE_TAG_POWER, "DisplayOffloader#startOffload");

        try {

            return mIsActive = mDisplayOffloader.startOffload();

            mIsActive = mDisplayOffloader.startOffload();

            if (DEBUG) {

                Slog.d(TAG, "startOffload = " + mIsActive);

            }

            return mIsActive;

        } finally {

            Trace.traceEnd(Trace.TRACE_TAG_POWER);

        }
@@ -110,6 +123,9 @@ public class DisplayOffloadSessionImpl implements DisplayManagerInternal.Display 
        try {

            mDisplayOffloader.stopOffload();

            mIsActive = false;

            if (DEBUG) {

                Slog.i(TAG, "stopOffload");

            }

        } finally {

            Trace.traceEnd(Trace.TRACE_TAG_POWER);

        }

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

+27 −2
Original line number Diff line number Diff line
@@ -791,6 +791,7 @@ 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)
@@ -1278,7 +1279,7 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call 


    private void updatePowerStateInternal() {

        // Update the power state request.

        final boolean mustNotify;

        boolean mustNotify = false;

        final int previousPolicy;

        boolean mustInitialize = false;

        mBrightnessReasonTemp.set(null);
@@ -1326,6 +1327,30 @@ 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/feature/DisplayManagerFlags.java

+10 −0
Original line number Diff line number Diff line
@@ -169,6 +169,11 @@ public class DisplayManagerFlags { 
            Flags::enableSynthetic60hzModes

    );



    private final FlagState mOffloadDozeOverrideHoldsWakelock = new FlagState(

            Flags.FLAG_OFFLOAD_DOZE_OVERRIDE_HOLDS_WAKELOCK,

            Flags::offloadDozeOverrideHoldsWakelock

    );



    /**

     * @return {@code true} if 'port' is allowed in display layout configuration file.

     */
@@ -331,6 +336,10 @@ public class DisplayManagerFlags { 
        return mPeakRefreshRatePhysicalLimit.isEnabled();

    }



    public boolean isOffloadDozeOverrideHoldsWakelockEnabled() {

        return mOffloadDozeOverrideHoldsWakelock.isEnabled();

    }



    /**

     * @return Whether to ignore preferredRefreshRate app request or not

     */
@@ -376,6 +385,7 @@ public class DisplayManagerFlags { 
        pw.println(" " + mPeakRefreshRatePhysicalLimit);

        pw.println(" " + mIgnoreAppPreferredRefreshRate);

        pw.println(" " + mSynthetic60hzModes);

        pw.println(" " + mOffloadDozeOverrideHoldsWakelock);

    }



    private static class FlagState {

services/core/java/com/android/server/display/feature/display_flags.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -278,3 +278,13 @@ flag { 
    }

}



flag {

    name: "offload_doze_override_holds_wakelock"

    namespace: "display_manager"

    description: "DisplayPowerController holds a suspend-blocker while changing the display state on behalf of offload doze override."

    bug: "338403827"

    is_fixed_read_only: true

    metadata {

      purpose: PURPOSE_BUGFIX

    }

}

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

+7 −0
Original line number Diff line number Diff line
@@ -1477,6 +1477,8 @@ public final class DisplayPowerControllerTest { 


    @Test

    public void testDozeScreenStateOverride_toSupportedOffloadStateFromDoze_DisplayStateChanges() {

        when(mDisplayManagerFlagsMock.isOffloadDozeOverrideHoldsWakelockEnabled()).thenReturn(true);



        // set up.

        int initState = Display.STATE_DOZE;

        int supportedTargetState = Display.STATE_DOZE_SUSPEND;
@@ -1495,10 +1497,15 @@ public final class DisplayPowerControllerTest { 
        mHolder.dpc.requestPowerState(dpr, /* waitForNegativeProximity= */ false);

        advanceTime(1); // Run updatePowerState



        reset(mHolder.wakelockController);

        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.displayPowerState)

                .setScreenState(supportedTargetState, Display.STATE_REASON_DEFAULT_POLICY);

    }