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

Commit 161feafe authored by Gil Dekel's avatar Gil Dekel
Browse files

SF: Reject hotplugs on invalid or duplicate ports 

When a HWC returns an invalid or duplicate port that collides with an
existing active port, the end result is display identification confusion
in higher layers of the stack. This is especially bad when the confusion
is with the internal/primary display and causes it to malfunction.

Reject hotplugs in which the reported port from HWC is already active.

Flag: EXEMPT bugfix
Bug: 383430671
Test: Display{Id | Identification}Test && libsurfaceflinger_unittest
Change-Id: Id3569ef1d973f4ace51d14c7e3cc9aef17630b22
parent 3f9ee01a

libs/ui/include/ui/DisplayMap.h

+3 −0
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ 


#include <ftl/small_map.h>

#include <ftl/small_vector.h>

#include <ftl/unit.h>



namespace android::ui {
@@ -30,6 +31,8 @@ using DisplayMap = ftl::SmallMap<Key, Value, kDisplayCapacity>; 
constexpr size_t kPhysicalDisplayCapacity = 3;

template <typename Key, typename Value>

using PhysicalDisplayMap = ftl::SmallMap<Key, Value, kPhysicalDisplayCapacity>;

template <typename Key>

using PhysicalDisplaySet = ftl::SmallMap<Key, ftl::Unit, kPhysicalDisplayCapacity>;



template <typename T>

using DisplayVector = ftl::SmallVector<T, kDisplayCapacity>;

services/surfaceflinger/DisplayDevice.h

+1 −0
Original line number Diff line number Diff line
@@ -260,6 +260,7 @@ struct DisplayDeviceState { 
    struct Physical {

        PhysicalDisplayId id;

        hardware::graphics::composer::hal::HWDisplayId hwcDisplayId;

        uint8_t port;

        DisplayModePtr activeMode;



        bool operator==(const Physical& other) const {

services/surfaceflinger/DisplayHardware/HWComposer.cpp

+19 −4
Original line number Diff line number Diff line
@@ -225,7 +225,11 @@ bool HWComposer::allocateVirtualDisplay(HalVirtualDisplayId displayId, ui::Size 
}



void HWComposer::allocatePhysicalDisplay(hal::HWDisplayId hwcDisplayId, PhysicalDisplayId displayId,

                                         std::optional<ui::Size> physicalSize) {

                                         uint8_t port, std::optional<ui::Size> physicalSize) {

    LOG_ALWAYS_FATAL_IF(!mActivePorts.try_emplace(port).second,

                        "Cannot attach display %" PRIu64 " to an already active port %" PRIu8 ".",

                        hwcDisplayId, port);



    mPhysicalDisplayIdMap[hwcDisplayId] = displayId;



    if (!mPrimaryHwcDisplayId) {
@@ -239,6 +243,7 @@ void HWComposer::allocatePhysicalDisplay(hal::HWDisplayId hwcDisplayId, Physical 
    newDisplay->setConnected(true);

    newDisplay->setPhysicalSizeInMm(physicalSize);

    displayData.hwcDisplay = std::move(newDisplay);

    displayData.port = port;

}



int32_t HWComposer::getAttribute(hal::HWDisplayId hwcDisplayId, hal::HWConfigId configId,
@@ -758,6 +763,9 @@ void HWComposer::disconnectDisplay(HalDisplayId displayId) { 
    const auto hwcDisplayId = displayData.hwcDisplay->getId();



    mPhysicalDisplayIdMap.erase(hwcDisplayId);

    if (const auto port = displayData.port) {

        mActivePorts.erase(port.value());

    }

    mDisplayData.erase(displayId);



    // Reset the primary display ID if we're disconnecting it.
@@ -1123,8 +1131,15 @@ std::optional<hal::HWDisplayId> HWComposer::fromPhysicalDisplayId( 
    return {};

}



bool HWComposer::shouldIgnoreHotplugConnect(hal::HWDisplayId hwcDisplayId,

bool HWComposer::shouldIgnoreHotplugConnect(hal::HWDisplayId hwcDisplayId, uint8_t port,

                                            bool hasDisplayIdentificationData) const {

    if (mActivePorts.contains(port)) {

        ALOGE("Ignoring connection of display %" PRIu64 ". Port %" PRIu8

              " is already in active use.",

              hwcDisplayId, port);

        return true;

    }



    if (mHasMultiDisplaySupport && !hasDisplayIdentificationData) {

        ALOGE("Ignoring connection of display %" PRIu64 " without identification data",

              hwcDisplayId);
@@ -1170,7 +1185,7 @@ std::optional<DisplayIdentificationInfo> HWComposer::onHotplugConnect( 
                  mHasMultiDisplaySupport ? "generalized" : "legacy");

        }



        if (shouldIgnoreHotplugConnect(hwcDisplayId, hasDisplayIdentificationData)) {

        if (shouldIgnoreHotplugConnect(hwcDisplayId, port, hasDisplayIdentificationData)) {

            return {};

        }
@@ -1202,7 +1217,7 @@ std::optional<DisplayIdentificationInfo> HWComposer::onHotplugConnect( 
        if (info->preferredDetailedTimingDescriptor) {

            size = info->preferredDetailedTimingDescriptor->physicalSizeInMm;

        }

        allocatePhysicalDisplay(hwcDisplayId, info->id, size);

        allocatePhysicalDisplay(hwcDisplayId, info->id, info->port, size);

    }

    return info;

}

services/surfaceflinger/DisplayHardware/HWComposer.h

+7 −3
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ 
#include <ftl/expected.h>

#include <ftl/future.h>

#include <ui/DisplayIdentification.h>

#include <ui/DisplayMap.h>

#include <ui/FenceTime.h>

#include <ui/PictureProfileHandle.h>
@@ -144,7 +145,7 @@ public: 
    // supported by the HWC can be queried in advance, but allocation may fail for other reasons.

    virtual bool allocateVirtualDisplay(HalVirtualDisplayId, ui::Size, ui::PixelFormat*) = 0;



    virtual void allocatePhysicalDisplay(hal::HWDisplayId, PhysicalDisplayId,

    virtual void allocatePhysicalDisplay(hal::HWDisplayId, PhysicalDisplayId, uint8_t port,

                                         std::optional<ui::Size> physicalSize) = 0;



    // Attempts to create a new layer on this display
@@ -362,7 +363,7 @@ public: 
    bool allocateVirtualDisplay(HalVirtualDisplayId, ui::Size, ui::PixelFormat*) override;



    // Called from SurfaceFlinger, when the state for a new physical display needs to be recreated.

    void allocatePhysicalDisplay(hal::HWDisplayId, PhysicalDisplayId,

    void allocatePhysicalDisplay(hal::HWDisplayId, PhysicalDisplayId, uint8_t port,

                                 std::optional<ui::Size> physicalSize) override;



    // Attempts to create a new layer on this display
@@ -525,6 +526,7 @@ private: 


    struct DisplayData {

        std::unique_ptr<HWC2::Display> hwcDisplay;

        std::optional<uint8_t> port; // Set on hotplug for physical displays



        sp<Fence> lastPresentFence = Fence::NO_FENCE; // signals when the last set op retires

        nsecs_t lastPresentTimestamp = 0;
@@ -542,7 +544,8 @@ private: 


    std::optional<DisplayIdentificationInfo> onHotplugConnect(hal::HWDisplayId);

    std::optional<DisplayIdentificationInfo> onHotplugDisconnect(hal::HWDisplayId);

    bool shouldIgnoreHotplugConnect(hal::HWDisplayId, bool hasDisplayIdentificationData) const;

    bool shouldIgnoreHotplugConnect(hal::HWDisplayId, uint8_t port,

                                    bool hasDisplayIdentificationData) const;



    aidl::android::hardware::graphics::composer3::DisplayConfiguration::Dpi

    getEstimatedDotsPerInchFromSize(uint64_t hwcDisplayId, const HWCDisplayMode& hwcMode) const;
@@ -564,6 +567,7 @@ private: 
    void loadHdrConversionCapabilities();



    std::unordered_map<HalDisplayId, DisplayData> mDisplayData;

    ui::PhysicalDisplaySet<uint8_t> mActivePorts;



    std::unique_ptr<android::Hwc2::Composer> mComposer;

    std::unordered_set<aidl::android::hardware::graphics::composer3::Capability> mCapabilities;

services/surfaceflinger/SurfaceFlinger.cpp

+7 −5
Original line number Diff line number Diff line
@@ -3718,6 +3718,7 @@ std::optional<DisplayModeId> SurfaceFlinger::processHotplugConnect(PhysicalDispl 
    if (const auto displayOpt = mPhysicalDisplays.get(displayId)) {

        const auto& display = displayOpt->get();

        const auto& snapshot = display.snapshot();

        const uint8_t port = snapshot.port();



        std::optional<DeviceProductInfo> deviceProductInfo;

        if (getHwComposer().updatesDeviceProductInfoOnHotplugReconnect()) {
@@ -3729,14 +3730,14 @@ std::optional<DisplayModeId> SurfaceFlinger::processHotplugConnect(PhysicalDispl 
        // Use the cached port via snapshot because we are updating an existing

        // display on reconnect.

        const auto it =

                mPhysicalDisplays.try_replace(displayId, display.token(), displayId,

                                              snapshot.port(), snapshot.connectionType(),

                                              std::move(displayModes), std::move(colorModes),

                                              std::move(deviceProductInfo));

                mPhysicalDisplays.try_replace(displayId, display.token(), displayId, port,

                                              snapshot.connectionType(), std::move(displayModes),

                                              std::move(colorModes), std::move(deviceProductInfo));



        auto& state = mCurrentState.displays.editValueFor(it->second.token());

        state.sequenceId = DisplayDeviceState{}.sequenceId; // Generate new sequenceId.

        state.physical->activeMode = std::move(activeMode);

        state.physical->port = port;

        ALOGI("Reconnecting %s", displayString);

        return activeModeId;

    }
@@ -3752,6 +3753,7 @@ std::optional<DisplayModeId> SurfaceFlinger::processHotplugConnect(PhysicalDispl 
    DisplayDeviceState state;

    state.physical = {.id = displayId,

                      .hwcDisplayId = hwcDisplayId,

                      .port = info.port,

                      .activeMode = std::move(activeMode)};

    if (mIsHdcpViaNegVsync) {

        state.isSecure = connectionType == ui::DisplayConnectionType::Internal;
@@ -4067,7 +4069,7 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken, 


        if (const auto& physical = currentState.physical) {

            getHwComposer().allocatePhysicalDisplay(physical->hwcDisplayId, physical->id,

                                                    /*physicalSize=*/std::nullopt);

                                                    physical->port, /*physicalSize=*/std::nullopt);

        }



        processDisplayAdded(displayToken, currentState);