fix(i2c_master): Do not busy wait on sync transactions

This commit introduces a private member `i2c_master_bus_t::in_progress_semphr`
which allows `i2c_master_transmit` and related functions to give up their
timeslice so other tasks can proceed.

Without this commit, one must delay at least one tick via `vTaskDelay(1)` to
prevent trigging the idle watchdog, thus limiting the maximum I2C transaction
rate to about the speed of `CONFIG_FREERTOS_HZ`.

With this commit, over 6600 samples/sec are possible and the tick delay is
unnecessary.

CPU usage below was measured using `vTaskGetRunTimeStats()`.  The FreeRTOS %CPU
numbers seem to increase over time and level out after a time, so CPU usage was
sampled after 5 seconds across each test for consistency across measurements.
Measurements were taken with `CONFIG_FREERTOS_HZ` set to 100Hz and 1000Hz:

Before:
	 100Hz: samples/sec=  99 with  1% CPU ( 99.0 samples/%cpu)
	1000Hz: samples/sec= 995 with 10% CPU ( 99.5 samples/%cpu)

After:
	 100Hz: samples/sec=6637 with 23% CPU (288.6 samples/%cpu)
	1000Hz: samples/sec=6610 with 24% CPU (275.4 samples/%cpu)

Closes: espressif#13137

Signed-off-by: Eric Wheeler <[email protected]>

components/esp_driver_i2c/i2c_master.c

@@ -432,10 +432,28 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t
             s_i2c_start_end_command(i2c_master, i2c_operation, &address_fill, NULL);
         }
     }
+
+    // Flush the in_progress semaphore in case a previous request timed out but
+    // the request still completed later in the ISR. Such a case would cause
+    // the ISR to Give but would never have been Taken.  This zero-wait Take
+    // loop will reset to a zero-count state:
+    while (xSemaphoreTake(i2c_master->in_progress_semphr, 0) == pdTRUE) {}
+
+    // Start the transaction:
     i2c_hal_master_trans_start(hal);
 
-    // For blocking implementation, we must wait `done` interrupt to update the status.
-    while (i2c_master->trans_done == false) {};
+    // For blocking implementation, the `i2c_master_isr_handler_default` will
+    // unlock (Give) the `in_progress_semphr` when the transaction is complete.
+    // If the ISR is already done, then this xSemaphoreTake will fall through
+    // immediately without blocking:
+    if (xSemaphoreTake(i2c_master->in_progress_semphr, ticks_to_wait) == pdTRUE)
+        assert(i2c_master->trans_done == true);
+    else {
+        i2c_master->cmd_idx = 0;
+        i2c_master->trans_idx = 0;
+        i2c_master->status = I2C_STATUS_TIMEOUT;
+    }
+
     if (i2c_master->status != I2C_STATUS_ACK_ERROR && i2c_master->status != I2C_STATUS_TIMEOUT) {
         atomic_store(&i2c_master->status, I2C_STATUS_DONE);
     }
@@ -645,7 +663,15 @@ static void IRAM_ATTR i2c_master_isr_handler_default(void *arg)
             }
         }
     } else {
+        // If the sync transaction is done, then signal s_i2c_send_commands to proceed:
+        if (i2c_master->trans_done == true)
+            xSemaphoreGiveFromISR(i2c_master->in_progress_semphr, &HPTaskAwoken);
+
         xSemaphoreGiveFromISR(i2c_master->cmd_semphr, &HPTaskAwoken);
+
+        // True if Give resulted in a high priority task to wake, or when done
+        // because s_i2c_send_commands needs to wake:
+        HPTaskAwoken = HPTaskAwoken || i2c_master->trans_done;
     }
 
     if (HPTaskAwoken == pdTRUE) {
@@ -682,6 +708,10 @@ static esp_err_t i2c_master_bus_destroy(i2c_master_bus_handle_t bus_handle)
                 vSemaphoreDeleteWithCaps(i2c_master->cmd_semphr);
                 i2c_master->cmd_semphr = NULL;
             }
+            if (i2c_master->in_progress_semphr) {
+                vSemaphoreDeleteWithCaps(i2c_master->in_progress_semphr);
+                i2c_master->in_progress_semphr = NULL;
+            }
             if (i2c_master->queues_storage) {
                 free(i2c_master->queues_storage);
             }
@@ -829,6 +859,10 @@ esp_err_t i2c_new_master_bus(const i2c_master_bus_config_t *bus_config, i2c_mast
     i2c_master->cmd_semphr = xSemaphoreCreateBinaryWithCaps(I2C_MEM_ALLOC_CAPS);
     ESP_GOTO_ON_FALSE(i2c_master->cmd_semphr, ESP_ERR_NO_MEM, err, TAG, "no memory for i2c semaphore struct");
 
+    // We do not Give on initialization because the ISR will Give when a sync operation is complete:
+    i2c_master->in_progress_semphr = xSemaphoreCreateBinaryWithCaps(I2C_MEM_ALLOC_CAPS);
+    ESP_GOTO_ON_FALSE(i2c_master->in_progress_semphr, ESP_ERR_NO_MEM, err, TAG, "no memory for i2c progress mutex");
+
     portENTER_CRITICAL(&i2c_master->base->spinlock);
     i2c_ll_clear_intr_mask(hal->dev, I2C_LL_MASTER_EVENT_INTR);
     portEXIT_CRITICAL(&i2c_master->base->spinlock);

components/esp_driver_i2c/i2c_private.h

@@ -131,6 +131,7 @@ struct i2c_master_bus_t {
     i2c_operation_t i2c_ops[I2C_STATIC_OPERATION_ARRAY_MAX];         // I2C operation array
     _Atomic uint16_t trans_idx;                                      // Index of I2C transaction command.
     SemaphoreHandle_t cmd_semphr;                                    // Semaphore between task and interrupt, using for synchronizing ISR and I2C task.
+    SemaphoreHandle_t in_progress_semphr;                            // Used to prevent busy-waiting during IO
     uint32_t read_buf_pos;                                           // Read buffer position
     bool contains_read;                                              // Whether command array includes read operation, true: yes, otherwise, false.
     uint32_t read_len_static;                                        // Read static buffer length