Merge "SF: Reject hotplug if display modes failed to load" (add53b16) · Commits · e / os / platform_frameworks_native

services/surfaceflinger/SurfaceFlinger.cpp

+15 −6

Original line number	Diff line number	Diff line
		@@ -2581,10 +2581,9 @@ std::pair<DisplayModes, DisplayModePtr> SurfaceFlinger::loadDisplayModes(
		do {
		hwcModes = getHwComposer().getModes(displayId);
		activeModeHwcId = getHwComposer().getActiveMode(displayId);
		LOG_ALWAYS_FATAL_IF(!activeModeHwcId, "HWC returned no active mode");

		const auto isActiveMode = [activeModeHwcId](const HWComposer::HWCDisplayMode& mode) {
		return mode.hwcId == *activeModeHwcId;
		return mode.hwcId == activeModeHwcId;
		};

		if (std::any_of(hwcModes.begin(), hwcModes.end(), isActiveMode)) {
		@@ -2592,10 +2591,14 @@ std::pair<DisplayModes, DisplayModePtr> SurfaceFlinger::loadDisplayModes(
		}
		} while (++attempt < kMaxAttempts);

		LOG_ALWAYS_FATAL_IF(attempt == kMaxAttempts,
		"After %d attempts HWC still returns an active mode which is not"
		" supported. Active mode ID = %" PRIu64 ". Supported modes = %s",
		kMaxAttempts, *activeModeHwcId, base::Join(hwcModes, ", ").c_str());
		if (attempt == kMaxAttempts) {
		const std::string activeMode =
		activeModeHwcId ? std::to_string(*activeModeHwcId) : "unknown"s;
		ALOGE("HWC failed to report an active mode that is supported: activeModeHwcId=%s, "
		"hwcModes={%s}",
		activeMode.c_str(), base::Join(hwcModes, ", ").c_str());
		return {};
		}

		DisplayModes oldModes;
		if (const auto token = getPhysicalDisplayTokenLocked(displayId)) {
		@@ -2653,6 +2656,12 @@ void SurfaceFlinger::processDisplayHotplugEventsLocked() {

		if (event.connection == hal::Connection::CONNECTED) {
		auto [supportedModes, activeMode] = loadDisplayModes(displayId);
		if (!activeMode) {
		// TODO(b/241286153): Report hotplug failure to the framework.
		ALOGE("Failed to hotplug display %s", to_string(displayId).c_str());
		getHwComposer().disconnectDisplay(displayId);
		continue;
		}

		if (!token) {
		ALOGV("Creating display %s", to_string(displayId).c_str());

services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp

+6 −1

Original line number	Diff line number	Diff line
		@@ -168,7 +168,12 @@ sp<DisplayDevice> DisplayTransactionTest::injectDefaultInternalDisplay(
		}

		bool DisplayTransactionTest::hasPhysicalHwcDisplay(HWDisplayId hwcDisplayId) const {
		return mFlinger.hwcPhysicalDisplayIdMap().count(hwcDisplayId) == 1;
		const auto& map = mFlinger.hwcPhysicalDisplayIdMap();

		const auto it = map.find(hwcDisplayId);
		if (it == map.end()) return false;

		return mFlinger.hwcDisplayData().count(it->second) == 1;
		}

		bool DisplayTransactionTest::hasTransactionFlagSet(int32_t flag) const {

services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h

+11 −7

Original line number	Diff line number	Diff line
		@@ -434,14 +434,18 @@ struct HwcDisplayVariant {
		}
		}

		template <bool kFailedHotplug = false>
		static void setupHwcHotplugCallExpectations(DisplayTransactionTest* test) {
		if constexpr (!kFailedHotplug) {
		constexpr auto CONNECTION_TYPE =
		PhysicalDisplay::CONNECTION_TYPE == ui::DisplayConnectionType::Internal
		? IComposerClient::DisplayConnectionType::INTERNAL
		: IComposerClient::DisplayConnectionType::EXTERNAL;

		EXPECT_CALL(*test->mComposer, getDisplayConnectionType(HWC_DISPLAY_ID, _))
		.WillOnce(DoAll(SetArgPointee<1>(CONNECTION_TYPE), Return(hal::V2_4::Error::NONE)));
		.WillOnce(DoAll(SetArgPointee<1>(CONNECTION_TYPE),
		Return(hal::V2_4::Error::NONE)));
		}

		EXPECT_CALL(*test->mComposer, setClientTargetSlotCount(_))
		.WillOnce(Return(hal::Error::NONE));

services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp

+31 −2

Original line number	Diff line number	Diff line
		@@ -20,7 +20,6 @@
		#include "DisplayTransactionTestHelpers.h"

		namespace android {
		namespace {

		class HotplugTest : public DisplayTransactionTest {};

		@@ -107,5 +106,35 @@ TEST_F(HotplugTest, processesEnqueuedEventsIfCalledOnMainThread) {
		EXPECT_TRUE(mFlinger.mutablePendingHotplugEvents().empty());
		}

		} // namespace
		TEST_F(HotplugTest, rejectsHotplugIfFailedToLoadDisplayModes) {
		// Inject a primary display.
		PrimaryDisplayVariant::injectHwcDisplay(this);

		using ExternalDisplay = ExternalDisplayVariant;
		constexpr bool kFailedHotplug = true;
		ExternalDisplay::setupHwcHotplugCallExpectations<kFailedHotplug>(this);

		// Simulate a connect event that fails to load display modes due to HWC already having
		// disconnected the display but SF yet having to process the queued disconnect event.
		EXPECT_CALL(*mComposer, getActiveConfig(ExternalDisplay::HWC_DISPLAY_ID, _))
		.WillRepeatedly(Return(Error::BAD_DISPLAY));

		// TODO(b/241286146): Remove this unnecessary call.
		EXPECT_CALL(*mComposer,
		setVsyncEnabled(ExternalDisplay::HWC_DISPLAY_ID, IComposerClient::Vsync::DISABLE))
		.WillOnce(Return(Error::NONE));

		ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);
		mFlinger.processDisplayHotplugEvents();

		// The hotplug should be rejected, so no HWComposer::DisplayData should be created.
		EXPECT_FALSE(hasPhysicalHwcDisplay(ExternalDisplay::HWC_DISPLAY_ID));

		// Disconnecting a display that does not exist should be a no-op.
		ExternalDisplay::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
		mFlinger.processDisplayHotplugEvents();

		EXPECT_FALSE(hasPhysicalHwcDisplay(ExternalDisplay::HWC_DISPLAY_ID));
		}

		} // namespace android

services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h

+6 −0

Original line number	Diff line number	Diff line
		@@ -371,6 +371,10 @@ public:
		mFlinger->onComposerHalHotplug(hwcDisplayId, connection);
		}

		void processDisplayHotplugEvents() {
		FTL_FAKE_GUARD(mFlinger->mStateLock, mFlinger->processDisplayHotplugEventsLocked());
		}

		auto setDisplayStateLocked(const DisplayState& s) {
		Mutex::Autolock lock(mFlinger->mStateLock);
		return mFlinger->setDisplayStateLocked(s);
		@@ -499,7 +503,9 @@ public:
		const auto& currentState() const { return mFlinger->mCurrentState; }
		const auto& drawingState() const { return mFlinger->mDrawingState; }
		const auto& transactionFlags() const { return mFlinger->mTransactionFlags; }

		const auto& hwcPhysicalDisplayIdMap() const { return getHwComposer().mPhysicalDisplayIdMap; }
		const auto& hwcDisplayData() const { return getHwComposer().mDisplayData; }

		auto& mutableHasWideColorDisplay() { return SurfaceFlinger::hasWideColorDisplay; }