From 574d046bd7c3a2d81f48caf3881a61ad0d283e9a Mon Sep 17 00:00:00 2001 From: Badhri Jagan Sridharan Date: Wed, 7 Jun 2023 07:27:33 +0000 Subject: [PATCH 1/2] Usb: Check for displayport when booting When booting check whether displayport driver has been probed and signal hpd events as needed. Pin config and orientation needs to be queried before signalling hpd. Bug: 282223693 Change-Id: Iddb0921b363e49c41c0f6f635887b4daf4d19561 --- usb/usb/Usb.cpp | 48 ++++++++++++++++++++++++++++++++++++++++-------- usb/usb/Usb.h | 1 + 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/usb/usb/Usb.cpp b/usb/usb/Usb.cpp index c9955423..11079eea 100644 --- a/usb/usb/Usb.cpp +++ b/usb/usb/Usb.cpp @@ -817,11 +817,22 @@ done: void queryVersionHelper(android::hardware::usb::Usb *usb, std::vector *currentPortStatus) { Status status; + string displayPortUsbPath; + pthread_mutex_lock(&usb->mLock); status = getPortStatusHelper(usb, currentPortStatus); queryMoistureDetectionStatus(currentPortStatus); queryPowerTransferStatus(currentPortStatus); queryNonCompliantChargerStatus(currentPortStatus); + pthread_mutex_lock(&usb->mDisplayPortLock); + if (!usb->mDisplayPortFirstSetupDone && + usb->getDisplayPortUsbPathHelper(&displayPortUsbPath) == Status::SUCCESS) { + + ALOGI("usbdp: boot with display connected or usb hal restarted"); + usb->setupDisplayPortPoll(); + } + pthread_mutex_unlock(&usb->mDisplayPortLock); + if (usb->mCallback != NULL) { ScopedAStatus ret = usb->mCallback->notifyPortStatusChange(*currentPortStatus, status); @@ -1184,7 +1195,7 @@ Status Usb::writeDisplayPortAttribute(string attribute, string usb_path) { uint32_t temp; if (!::android::base::ParseUint(Trim(attrUsb), &temp)) { ALOGE("usbdp: failed parsing irq_hpd_count:%s", attrUsb.c_str()); - return Status::SUCCESS; + return Status::ERROR; } // Used to cache the values read from tcpci's irq_hpd_count. // Update drm driver when cached value is not the same as the read value. @@ -1203,7 +1214,7 @@ Status Usb::writeDisplayPortAttribute(string attribute, string usb_path) { } else { // Don't write anything ALOGI("usbdp: Pin config not yet chosen, nothing written."); - return Status::SUCCESS; + return Status::ERROR; } } @@ -1250,6 +1261,11 @@ void *displayPortPollWork(void *param) { string tcpcI2cBus, linkPath; ::aidl::android::hardware::usb::Usb *usb = (::aidl::android::hardware::usb::Usb *)param; + if (usb->mDisplayPortPollRunning) { + ALOGI("usbdp: displayPortPollWork already running. Shutting down duplicate."); + return NULL; + } + if (usb->getDisplayPortUsbPathHelper(&displayPortUsbPath) == Status::ERROR) { ALOGE("usbdp: could not locate usb displayport directory"); goto usb_path_error; @@ -1265,7 +1281,7 @@ void *displayPortPollWork(void *param) { getI2cBusHelper(&tcpcI2cBus); irqHpdCountPath = kI2CPath + tcpcI2cBus + "/" + tcpcI2cBus + kIrqHpdCounPath; - ALOGI("udbdp: irqHpdCountPath:%s", irqHpdCountPath.c_str()); + ALOGI("usbdp: irqHpdCountPath:%s", irqHpdCountPath.c_str()); epoll_fd = epoll_create(64); if (epoll_fd == -1) { @@ -1336,14 +1352,28 @@ void *displayPortPollWork(void *param) { if (events[n].data.fd == hpd_fd) { if (!pinSet || !orientationSet) { ALOGW("usbdp: HPD may be set before pin_assignment and orientation"); + if (!pinSet && + usb->writeDisplayPortAttribute("pin_assignment", pinAssignmentPath) == + Status::SUCCESS) { + pinSet = true; + } + if (!orientationSet && + usb->writeDisplayPortAttribute("orientation", orientationPath) == + Status::SUCCESS) { + orientationSet = true; + } } usb->writeDisplayPortAttribute("hpd", hpdPath); } else if (events[n].data.fd == pin_fd) { - usb->writeDisplayPortAttribute("pin_assignment", pinAssignmentPath); - pinSet = true; + if (usb->writeDisplayPortAttribute("pin_assignment", pinAssignmentPath) == + Status::SUCCESS) { + pinSet = true; + } } else if (events[n].data.fd == orientation_fd) { - usb->writeDisplayPortAttribute("orientation", orientationPath); - orientationSet = true; + if (usb->writeDisplayPortAttribute("orientation", orientationPath) == + Status::SUCCESS) { + orientationSet = true; + } } else if (events[n].data.fd == usb->mDisplayPortEventPipe) { uint64_t flag = 0; if (!read(usb->mDisplayPortEventPipe, &flag, sizeof(flag))) { @@ -1387,6 +1417,7 @@ void Usb::setupDisplayPortPoll() { write(mDisplayPortEventPipe, &flag, sizeof(flag)); destroyDisplayPortThread = false; + mDisplayPortFirstSetupDone = true; /* * Create a background thread to poll DisplayPort system files @@ -1417,7 +1448,8 @@ void Usb::shutdownDisplayPortPoll() { // Determine if should shutdown thread // getDisplayPortUsbPathHelper locates a DisplayPort directory, no need to double check // directory. - if (getDisplayPortUsbPathHelper(&displayPortUsbPath) == Status::SUCCESS) { + if (getDisplayPortUsbPathHelper(&displayPortUsbPath) == Status::SUCCESS || + !mDisplayPortPollRunning) { return; } diff --git a/usb/usb/Usb.h b/usb/usb/Usb.h index cdac4b50..1ad66bbb 100644 --- a/usb/usb/Usb.h +++ b/usb/usb/Usb.h @@ -106,6 +106,7 @@ struct Usb : public BnUsb { bool mUsbDataEnabled; // True when mDisplayPortPoll pthread is running volatile bool mDisplayPortPollRunning; + volatile bool mDisplayPortFirstSetupDone; // Used to cache the values read from tcpci's irq_hpd_count. // Update drm driver when cached value is not the same as the read value. uint32_t mIrqHpdCountCache; From 78f6294e381ca4df70b28d1d0f07aff77b73574c Mon Sep 17 00:00:00 2001 From: RD Babiera Date: Wed, 14 Jun 2023 01:05:08 +0000 Subject: [PATCH 2/2] Usb: shutdown old displayport poll thread when multiple start up Better enforces that only one DisplayPort poll thread should be active at once. If a new call to setup is made from additional bind uevents before a disconnect occurs, tells the old thread to shutdown and awaits signaling before a new worker thread spins up. Test: manual testing on device Bug: 286593610 Change-Id: I1707970c500915fd4abb161d0b3a424bbb2bddca --- usb/usb/Usb.cpp | 145 +++++++++++++++++++++++++++++++++++------------- usb/usb/Usb.h | 7 ++- 2 files changed, 112 insertions(+), 40 deletions(-) diff --git a/usb/usb/Usb.cpp b/usb/usb/Usb.cpp index 11079eea..74070770 100644 --- a/usb/usb/Usb.cpp +++ b/usb/usb/Usb.cpp @@ -452,6 +452,9 @@ Usb::Usb() ZoneInfo(TemperatureType::UNKNOWN, kThermalZoneForTempReadSecondary2, ThrottlingSeverity::NONE)}, kSamplingIntervalSec), mUsbDataEnabled(true), + mDisplayPortPollRunning(false), + mDisplayPortPollStarting(false), + mDisplayPortCVLock(PTHREAD_MUTEX_INITIALIZER), mDisplayPortLock(PTHREAD_MUTEX_INITIALIZER) { pthread_condattr_t attr; if (pthread_condattr_init(&attr)) { @@ -466,6 +469,10 @@ Usb::Usb() ALOGE("pthread_cond_init failed: %s", strerror(errno)); abort(); } + if (pthread_cond_init(&mDisplayPortCV, &attr)) { + ALOGE("usbdp: pthread_cond_init failed: %s", strerror(errno)); + abort(); + } if (pthread_condattr_destroy(&attr)) { ALOGE("pthread_condattr_destroy failed: %s", strerror(errno)); abort(); @@ -1005,7 +1012,7 @@ static void uevent_event(uint32_t /*epevents*/, struct data *payload) { pthread_mutex_unlock(&payload->usb->mDisplayPortLock); } else if (uevent_type == UeventType::CHANGE) { pthread_mutex_lock(&payload->usb->mDisplayPortLock); - payload->usb->shutdownDisplayPortPoll(); + payload->usb->shutdownDisplayPortPoll(false); pthread_mutex_unlock(&payload->usb->mDisplayPortLock); } break; @@ -1243,7 +1250,7 @@ bool Usb::determineDisplayPortRetry(string linkPath, string hpdPath) { static int displayPortPollOpenFileHelper(const char *file, int flags) { int fd = open(file, flags); if (fd == -1) { - ALOGE("usbdp: open at %s failed; errno=%d", file, errno); + ALOGE("usbdp: worker: open at %s failed; errno=%d", file, errno); } return fd; } @@ -1261,19 +1268,15 @@ void *displayPortPollWork(void *param) { string tcpcI2cBus, linkPath; ::aidl::android::hardware::usb::Usb *usb = (::aidl::android::hardware::usb::Usb *)param; - if (usb->mDisplayPortPollRunning) { - ALOGI("usbdp: displayPortPollWork already running. Shutting down duplicate."); - return NULL; - } + usb->mDisplayPortPollRunning = true; + usb->mDisplayPortPollStarting = false; if (usb->getDisplayPortUsbPathHelper(&displayPortUsbPath) == Status::ERROR) { - ALOGE("usbdp: could not locate usb displayport directory"); + ALOGE("usbdp: worker: could not locate usb displayport directory"); goto usb_path_error; } - usb->mDisplayPortPollRunning = true; - - ALOGI("usbdp: displayport usb path located at %s", displayPortUsbPath.c_str()); + ALOGI("usbdp: worker: displayport usb path located at %s", displayPortUsbPath.c_str()); hpdPath = displayPortUsbPath + "hpd"; pinAssignmentPath = displayPortUsbPath + "pin_assignment"; orientationPath = "/sys/class/typec/port0/orientation"; @@ -1281,11 +1284,11 @@ void *displayPortPollWork(void *param) { getI2cBusHelper(&tcpcI2cBus); irqHpdCountPath = kI2CPath + tcpcI2cBus + "/" + tcpcI2cBus + kIrqHpdCounPath; - ALOGI("usbdp: irqHpdCountPath:%s", irqHpdCountPath.c_str()); + ALOGI("usbdp: worker: irqHpdCountPath:%s", irqHpdCountPath.c_str()); epoll_fd = epoll_create(64); if (epoll_fd == -1) { - ALOGE("usbdp: epoll_create failed; errno=%d", errno); + ALOGE("usbdp: worker: epoll_create failed; errno=%d", errno); goto epoll_fd_error; } @@ -1317,23 +1320,23 @@ void *displayPortPollWork(void *param) { ev_link.data.fd = link_fd; if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, hpd_fd, &ev_hpd) == -1) { - ALOGE("usbdp: epoll_ctl failed to add hpd; errno=%d", errno); + ALOGE("usbdp: worker: epoll_ctl failed to add hpd; errno=%d", errno); goto error; } if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, pin_fd, &ev_pin) == -1) { - ALOGE("usbdp: epoll_ctl failed to add pin; errno=%d", errno); + ALOGE("usbdp: worker: epoll_ctl failed to add pin; errno=%d", errno); goto error; } if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, orientation_fd, &ev_orientation) == -1) { - ALOGE("usbdp: epoll_ctl failed to add orientation; errno=%d", errno); + ALOGE("usbdp: worker: epoll_ctl failed to add orientation; errno=%d", errno); goto error; } if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, link_fd, &ev_link) == -1) { - ALOGE("usbdp: epoll_ctl failed to add link status; errno=%d", errno); + ALOGE("usbdp: worker: epoll_ctl failed to add link status; errno=%d", errno); goto error; } if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, usb->mDisplayPortEventPipe, &ev_eventfd) == -1) { - ALOGE("usbdp: epoll_ctl failed to add orientation; errno=%d", errno); + ALOGE("usbdp: worker: epoll_ctl failed to add orientation; errno=%d", errno); goto error; } @@ -1344,14 +1347,14 @@ void *displayPortPollWork(void *param) { if (nevents == -1) { if (errno == EINTR) continue; - ALOGE("usbdp: epoll_wait failed; errno=%d", errno); + ALOGE("usbdp: worker: epoll_wait failed; errno=%d", errno); break; } for (int n = 0; n < nevents; n++) { if (events[n].data.fd == hpd_fd) { if (!pinSet || !orientationSet) { - ALOGW("usbdp: HPD may be set before pin_assignment and orientation"); + ALOGW("usbdp: worker: HPD may be set before pin_assignment and orientation"); if (!pinSet && usb->writeDisplayPortAttribute("pin_assignment", pinAssignmentPath) == Status::SUCCESS) { @@ -1379,15 +1382,15 @@ void *displayPortPollWork(void *param) { if (!read(usb->mDisplayPortEventPipe, &flag, sizeof(flag))) { if (errno == EAGAIN) continue; - ALOGI("usbdp: Shutdown eventfd read error"); + ALOGI("usbdp: worker: Shutdown eventfd read error"); goto error; } if (flag == DISPLAYPORT_SHUTDOWN_SET) { - ALOGI("usbdp: Shutdown eventfd triggered"); + ALOGI("usbdp: worker: Shutdown eventfd triggered"); destroyDisplayPortThread = true; break; } else if (flag == DISPLAYPORT_IRQ_HPD_COUNT_CHECK) { - ALOGI("usbdp: IRQ_HPD event through DISPLAYPORT_IRQ_HPD_COUNT_CHECK"); + ALOGI("usbdp: worker: IRQ_HPD event through DISPLAYPORT_IRQ_HPD_COUNT_CHECK"); usb->writeDisplayPortAttribute("irq_hpd_count", irqHpdCountPath); } } @@ -1408,58 +1411,122 @@ hpd_fd_error: epoll_fd_error: usb_path_error: usb->mDisplayPortPollRunning = false; - ALOGI("usbdp: Exiting worker thread"); + ALOGI("usbdp: worker: exiting worker thread"); return NULL; } +static struct timespec setTimespecTimer(int debounceMs) { + struct timespec to; + struct timespec now; + + clock_gettime(CLOCK_MONOTONIC, &now); + to.tv_nsec = now.tv_nsec + ((debounceMs % 1000) * 1000000); + to.tv_sec = now.tv_sec + (debounceMs / 1000); + if (to.tv_nsec >= 1000000000) { + to.tv_nsec -= 1000000000; + to.tv_sec += 1; + } + + return to; +} + void Usb::setupDisplayPortPoll() { uint64_t flag = DISPLAYPORT_SHUTDOWN_CLEAR; + mDisplayPortFirstSetupDone = true; + int ret; + ALOGI("usbdp: setup: beginning setup for displayport poll thread"); + + /* + * If thread is currently starting, then it hasn't setup DisplayPort fd's, and we can abandon + * this process. + */ + if (mDisplayPortPollStarting) { + ALOGI("usbdp: setup: abandoning poll thread because another startup is in progress"); + return; + } + + /* + * Check to see if thread is currently running. If it is, then we assume that it must have + * invalid DisplayPort fd's and the new thread takes over. + */ + if (mDisplayPortPollRunning) { + shutdownDisplayPortPoll(true); + pthread_mutex_lock(&mDisplayPortCVLock); + struct timespec to = setTimespecTimer(DISPLAYPORT_POLL_WAIT_MS); + ret = pthread_cond_timedwait(&mDisplayPortCV, &mDisplayPortCVLock, &to); + if (ret == ETIMEDOUT) { + ALOGI("usbdp: setup: Wait for poll to shutdown timed out, starting new poll anyways."); + } + pthread_mutex_unlock(&mDisplayPortCVLock); + } + + // Indicate that startup procedure is initiated (mutex protects two threads running setup at + // once) + mDisplayPortPollStarting = true; + + // Reset shutdown signals because shutdown() does not perform self clean-up write(mDisplayPortEventPipe, &flag, sizeof(flag)); destroyDisplayPortThread = false; - mDisplayPortFirstSetupDone = true; /* * Create a background thread to poll DisplayPort system files */ if (pthread_create(&mDisplayPortPoll, NULL, displayPortPollWork, this)) { - ALOGE("usbdp: failed to create displayport poll thread %d", errno); + ALOGE("usbdp: setup: failed to create displayport poll thread %d", errno); + goto error; } - ALOGI("usbdp: successfully started DisplayPort poll thread"); + ALOGI("usbdp: setup: successfully started displayport poll thread"); + return; + +error: + mDisplayPortPollStarting = false; return; } void Usb::shutdownDisplayPortPollHelper() { + uint64_t flag = DISPLAYPORT_SHUTDOWN_SET; + + // Write shutdown signal to child thread. + write(mDisplayPortEventPipe, &flag, sizeof(flag)); pthread_join(mDisplayPortPoll, NULL); + writeDisplayPortAttributeOverride("hpd", "0"); + pthread_mutex_lock(&mDisplayPortCVLock); + pthread_cond_signal(&mDisplayPortCV); + pthread_mutex_unlock(&mDisplayPortCVLock); } void *shutdownDisplayPortPollWork(void *param) { ::aidl::android::hardware::usb::Usb *usb = (::aidl::android::hardware::usb::Usb *)param; usb->shutdownDisplayPortPollHelper(); - ALOGI("usbdp: DisplayPort Thread Shutdown"); + ALOGI("usbdp: shutdown: displayport thread shutdown complete."); return NULL; } -void Usb::shutdownDisplayPortPoll() { - uint64_t flag = DISPLAYPORT_SHUTDOWN_SET; +void Usb::shutdownDisplayPortPoll(bool force) { string displayPortUsbPath; - // Determine if should shutdown thread - // getDisplayPortUsbPathHelper locates a DisplayPort directory, no need to double check - // directory. - if (getDisplayPortUsbPathHelper(&displayPortUsbPath) == Status::SUCCESS || - !mDisplayPortPollRunning) { + ALOGI("usbdp: shutdown: beginning shutdown for displayport poll thread"); + + /* + * Determine if should shutdown thread + * + * getDisplayPortUsbPathHelper locates a DisplayPort directory, no need to double check + * directory. + * + * Force is put in place to shutdown even when displayPortUsbPath is still present. + * Happens when back to back BIND events are sent and fds are no longer current. + */ + if (!mDisplayPortPollRunning || + (!force && getDisplayPortUsbPathHelper(&displayPortUsbPath) == Status::SUCCESS)) { return; } - // Shutdown thread, make sure to rewrite hpd because file no longer exists. - write(mDisplayPortEventPipe, &flag, sizeof(flag)); + // Shutdown is nonblocking to let other usb operations continue if (pthread_create(&mDisplayPortShutdownHelper, NULL, shutdownDisplayPortPollWork, this)) { - ALOGE("pthread creation failed %d", errno); + ALOGE("usbdp: shutdown: shutdown worker pthread creation failed %d", errno); } - writeDisplayPortAttributeOverride("hpd", "0"); - destroyDisplayPortThread = false; } } // namespace usb diff --git a/usb/usb/Usb.h b/usb/usb/Usb.h index 1ad66bbb..892d769e 100644 --- a/usb/usb/Usb.h +++ b/usb/usb/Usb.h @@ -61,6 +61,8 @@ constexpr char kGadgetName[] = "11210000.dwc3"; #define DISPLAYPORT_SHUTDOWN_SET 1 #define DISPLAYPORT_IRQ_HPD_COUNT_CHECK 3 +#define DISPLAYPORT_POLL_WAIT_MS 100 + struct Usb : public BnUsb { Usb(); @@ -84,7 +86,7 @@ struct Usb : public BnUsb { bool determineDisplayPortRetry(string linkPath, string hpdPath); void setupDisplayPortPoll(); void shutdownDisplayPortPollHelper(); - void shutdownDisplayPortPoll(); + void shutdownDisplayPortPoll(bool force); std::shared_ptr<::aidl::android::hardware::usb::IUsbCallback> mCallback; // Protects mCallback variable @@ -106,6 +108,9 @@ struct Usb : public BnUsb { bool mUsbDataEnabled; // True when mDisplayPortPoll pthread is running volatile bool mDisplayPortPollRunning; + volatile bool mDisplayPortPollStarting; + pthread_cond_t mDisplayPortCV; + pthread_mutex_t mDisplayPortCVLock; volatile bool mDisplayPortFirstSetupDone; // Used to cache the values read from tcpci's irq_hpd_count. // Update drm driver when cached value is not the same as the read value.