From e1ab132994795e6fb7d2cec0092a90c58b24cc07 Mon Sep 17 00:00:00 2001
From: Chris Paulo <chrispaulo@google.com>
Date: Fri, 19 Jan 2024 14:29:57 -0800
Subject: [PATCH] vibrator/cs40l26: Update ff_effect duration

Update the ff_effect duration to pass in 0 for compositions and RAM
indices.  This will allow firmware to take control of the playback
duration, as well as apply SVC braking as required.  This will also help
avoid ending the effect early which can cause ringing.

Bug: 317280093
Test: Verified perception and plotted acceleration
Change-Id: I048b2bbda04af2a627aa8993aafc1f55ccf8fe13
Signed-off-by: Chris Paulo <chrispaulo@google.com>
---
 vibrator/cs40l26/Hardware.h   |  2 +-
 vibrator/cs40l26/Vibrator.cpp | 24 ++++++++++++++++--------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/vibrator/cs40l26/Hardware.h b/vibrator/cs40l26/Hardware.h
index 7ad4017..22667c9 100644
--- a/vibrator/cs40l26/Hardware.h
+++ b/vibrator/cs40l26/Hardware.h
@@ -110,7 +110,7 @@ class HwApi : public Vibrator::HwApi, private HwApiBase {
         return true;
     }
     bool setFFEffect(int fd, struct ff_effect *effect, uint16_t timeoutMs) override {
-        if (((*effect).replay.length != timeoutMs) || (ioctl(fd, EVIOCSFF, effect) < 0)) {
+        if (ioctl(fd, EVIOCSFF, effect) < 0) {
             ALOGE("setFFEffect fail");
             return false;
         } else {
diff --git a/vibrator/cs40l26/Vibrator.cpp b/vibrator/cs40l26/Vibrator.cpp
index 7d5227e..bfb7cf1 100644
--- a/vibrator/cs40l26/Vibrator.cpp
+++ b/vibrator/cs40l26/Vibrator.cpp
@@ -363,7 +363,8 @@ Vibrator::Vibrator(std::unique_ptr<HwApi> hwApiDefault, std::unique_ptr<HwCal> h
             mFfEffects[effectIndex] = {
                     .type = FF_PERIODIC,
                     .id = -1,
-                    .replay.length = static_cast<uint16_t>(mEffectDurations[effectIndex]),
+                    // Length == 0 to allow firmware control of the duration
+                    .replay.length = 0,
                     .u.periodic.waveform = FF_CUSTOM,
                     .u.periodic.custom_data = new int16_t[2]{RAM_WVFRM_BANK, effectIndex},
                     .u.periodic.custom_len = FF_CUSTOM_DATA_LEN,
@@ -371,9 +372,11 @@ Vibrator::Vibrator(std::unique_ptr<HwApi> hwApiDefault, std::unique_ptr<HwCal> h
             // Bypass the waveform update due to different input name
             if ((strstr(inputEventName, "cs40l26") != nullptr) ||
                 (strstr(inputEventName, "cs40l26_dual_input") != nullptr)) {
+                // Let the firmware control the playback duration to avoid
+                // cutting any effect that is played short
                 if (!mHwApiDef->setFFEffect(
                             mInputFd, &mFfEffects[effectIndex],
-                            static_cast<uint16_t>(mFfEffects[effectIndex].replay.length))) {
+                            mEffectDurations[effectIndex])) {
                     ALOGE("Failed upload effect %d (%d): %s", effectIndex, errno, strerror(errno));
                 }
             }
@@ -403,7 +406,8 @@ Vibrator::Vibrator(std::unique_ptr<HwApi> hwApiDefault, std::unique_ptr<HwCal> h
                 mFfEffectsDual[effectIndex] = {
                         .type = FF_PERIODIC,
                         .id = -1,
-                        .replay.length = static_cast<uint16_t>(mEffectDurations[effectIndex]),
+                        // Length == 0 to allow firmware control of the duration
+                        .replay.length = 0,
                         .u.periodic.waveform = FF_CUSTOM,
                         .u.periodic.custom_data = new int16_t[2]{RAM_WVFRM_BANK, effectIndex},
                         .u.periodic.custom_len = FF_CUSTOM_DATA_LEN,
@@ -411,9 +415,11 @@ Vibrator::Vibrator(std::unique_ptr<HwApi> hwApiDefault, std::unique_ptr<HwCal> h
                 // Bypass the waveform update due to different input name
                 if ((strstr(inputEventName, "cs40l26") != nullptr) ||
                     (strstr(inputEventName, "cs40l26_dual_input") != nullptr)) {
+                    // Let the firmware control the playback duration to avoid
+                    // cutting any effect that is played short
                     if (!mHwApiDual->setFFEffect(
                                 mInputFdDual, &mFfEffectsDual[effectIndex],
-                                static_cast<uint16_t>(mFfEffectsDual[effectIndex].replay.length))) {
+                                mEffectDurations[effectIndex])) {
                         ALOGE("Failed upload flip's effect %d (%d): %s", effectIndex, errno,
                               strerror(errno));
                     }
@@ -795,9 +801,10 @@ ndk::ScopedAStatus Vibrator::compose(const std::vector<CompositeEffect> &composi
     if (header_count == dspmem_chunk_bytes(ch)) {
         return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
     } else {
-        mFfEffects[WAVEFORM_COMPOSE].replay.length = totalDuration;
+        // Composition duration should be 0 to allow firmware to play the whole effect
+        mFfEffects[WAVEFORM_COMPOSE].replay.length = 0;
         if (mIsDual) {
-            mFfEffectsDual[WAVEFORM_COMPOSE].replay.length = totalDuration;
+            mFfEffectsDual[WAVEFORM_COMPOSE].replay.length = 0;
         }
         return performEffect(WAVEFORM_MAX_INDEX /*ignored*/, VOLTAGE_SCALE_MAX /*ignored*/, ch,
                              callback);
@@ -1501,9 +1508,10 @@ ndk::ScopedAStatus Vibrator::getCompoundDetails(Effect effect, EffectStrength st
     }
 
     *outTimeMs = timeMs;
-    mFfEffects[WAVEFORM_COMPOSE].replay.length = static_cast<uint16_t>(timeMs);
+    // Compositions should have 0 duration
+    mFfEffects[WAVEFORM_COMPOSE].replay.length = 0;
     if (mIsDual) {
-        mFfEffectsDual[WAVEFORM_COMPOSE].replay.length = static_cast<uint16_t>(timeMs);
+        mFfEffectsDual[WAVEFORM_COMPOSE].replay.length = 0;
     }
 
     return ndk::ScopedAStatus::ok();