Skip to content

Commit

Permalink
i2c_master: use notifications to wait on sync tranasctions
Browse files Browse the repository at this point in the history
The previous commit uses semaphores, this commit converts semaphores to task
notifications because FreeRTOS claims they are faster, and indeed they are:

Notifications (this commit):
-  100Hz: samples/sec= 6654 with 18% CPU (efficiency: 369.7  samples/%cpu)
- 1000Hz: samples/sec= 6636 with 18% CPU (efficiency: 368.7 samples/%cpu)

Semaphores (previous commit):
-  100Hz: samples/sec= 6637 with 23% CPU (efficiency: 288.6 samples/%cpu)
- 1000Hz: samples/sec= 6610 with 24% CPU (efficiency: 275.4 samples/%cpu)

Signed-off-by: Eric Wheeler <[email protected]>
  • Loading branch information
KJ7LNW committed Mar 9, 2024
1 parent 65af55e commit d132c79
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 19 deletions.
29 changes: 11 additions & 18 deletions components/esp_driver_i2c/i2c_master.c
Original file line number Diff line number Diff line change
Expand Up @@ -433,27 +433,30 @@ static void s_i2c_send_commands(i2c_master_bus_handle_t i2c_master, TickType_t t
}
}

// Flush the in_progress semaphore in case a previous request timed out but
// Flush notifications 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) {}
i2c_master->caller_task = xTaskGetCurrentTaskHandle();
xTaskNotifyStateClear(i2c_master->caller_task);

// Start the transaction:
i2c_hal_master_trans_start(hal);

// 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
// notify this task when the transaction is complete.
// If the ISR is already done, then ulTaskNotifyTake will fall through
// immediately without blocking:
if (xSemaphoreTake(i2c_master->in_progress_semphr, ticks_to_wait) == pdTRUE)
if (ulTaskNotifyTake(pdTRUE, ticks_to_wait) != 0)
assert(i2c_master->trans_done == true);
else {
i2c_master->cmd_idx = 0;
i2c_master->trans_idx = 0;
i2c_master->status = I2C_STATUS_TIMEOUT;
}

i2c_master->caller_task = NULL;

if (i2c_master->status != I2C_STATUS_ACK_ERROR && i2c_master->status != I2C_STATUS_TIMEOUT) {
atomic_store(&i2c_master->status, I2C_STATUS_DONE);
}
Expand Down Expand Up @@ -664,14 +667,10 @@ 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:
portBASE_TYPE HPTaskAwokenInProgress = pdFALSE;
if (i2c_master->trans_done == true)
xSemaphoreGiveFromISR(i2c_master->in_progress_semphr, &HPTaskAwokenInProgress);
if (i2c_master->trans_done == true && likely(i2c_master->caller_task != NULL))
vTaskNotifyGiveFromISR(i2c_master->caller_task, &HPTaskAwoken);

xSemaphoreGiveFromISR(i2c_master->cmd_semphr, &HPTaskAwoken);

// True if either Give resulted in a high priority task to wake:
HPTaskAwoken = HPTaskAwoken || HPTaskAwokenInProgress;
}

if (HPTaskAwoken == pdTRUE) {
Expand Down Expand Up @@ -708,10 +707,6 @@ 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);
}
Expand Down Expand Up @@ -859,9 +854,7 @@ 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");
i2c_master->caller_task = NULL;

portENTER_CRITICAL(&i2c_master->base->spinlock);
i2c_ll_clear_intr_mask(hal->dev, I2C_LL_MASTER_EVENT_INTR);
Expand Down
2 changes: 1 addition & 1 deletion components/esp_driver_i2c/i2c_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +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
TaskHandle_t caller_task; // The task that started the transaction
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
Expand Down

0 comments on commit d132c79

Please sign in to comment.