docs(isp_awb): add isp awb programming guide

L-KAYA · L-KAYA · commit 4b3e01407635 · 2024-06-24T10:49:20.000+08:00
diff --git a/components/esp_driver_isp/include/driver/isp_af.h b/components/esp_driver_isp/include/driver/isp_af.h
@@ -178,7 +178,7 @@ typedef struct {
 /**
  * @brief Prototype of ISP AF Env detector event callback
  *
- * @param[in] af_ctrlr  ISP AF controller handle
+ * @param[in] af_ctrlr   ISP AF controller handle
  * @param[in] edata     ISP AF Env detector event data
  * @param[in] user_data User registered context, registered when in `esp_isp_af_env_detector_register_event_callbacks()`
  *
diff --git a/components/esp_driver_isp/include/driver/isp_types.h b/components/esp_driver_isp/include/driver/isp_types.h
@@ -30,6 +30,14 @@ typedef struct {
     float max;          ///< Maximum float value
 } isp_float_range_t;
 
+/**
+ * @brief ISP AF result
+ */
+typedef struct {
+    int definition[ISP_AF_WINDOW_NUM];    ///< Definition, it refers how clear and sharp an image is
+    int luminance[ISP_AF_WINDOW_NUM];     ///< Luminance, it refers how luminant an image is
+} isp_af_result_t;
+
 /**
  * @brief ISP AWB result
  */
diff --git a/components/esp_driver_isp/src/isp_awb.c b/components/esp_driver_isp/src/isp_awb.c
@@ -12,7 +12,7 @@
 #include "esp_check.h"
 #include "esp_heap_caps.h"
 #include "driver/isp_awb.h"
-#include "isp_internal.h"
+#include "esp_private/isp_private.h"
 
 typedef struct isp_awb_controller_t {
     isp_fsm_t                          fsm;
@@ -142,40 +142,43 @@ esp_err_t esp_isp_awb_controller_enable(isp_awb_ctlr_t awb_ctlr)
 {
     ESP_RETURN_ON_FALSE(awb_ctlr && awb_ctlr->isp_proc, ESP_ERR_INVALID_ARG, TAG, "invalid argument: null pointer");
     ESP_RETURN_ON_FALSE(awb_ctlr->fsm == ISP_FSM_INIT, ESP_ERR_INVALID_STATE, TAG, "controller isn't in init state");
+    awb_ctlr->fsm = ISP_FSM_ENABLE;
 
-    esp_intr_enable(awb_ctlr->intr_handle);
+    esp_err_t ret = ESP_OK;
+    ESP_GOTO_ON_ERROR(esp_intr_enable(awb_ctlr->intr_handle), err, TAG, "failed to enable the AWB interrupt");
     isp_ll_awb_clk_enable(awb_ctlr->isp_proc->hal.hw, true);
     isp_ll_enable_intr(awb_ctlr->isp_proc->hal.hw, ISP_LL_EVENT_AWB_MASK, true);
     xSemaphoreGive(awb_ctlr->stat_lock);
-    awb_ctlr->fsm = ISP_FSM_ENABLE;
 
-    return ESP_OK;
+    return ret;
+err:
+    awb_ctlr->fsm = ISP_FSM_INIT;
+    return ret;
 }
 
 esp_err_t esp_isp_awb_controller_disable(isp_awb_ctlr_t awb_ctlr)
 {
     ESP_RETURN_ON_FALSE(awb_ctlr && awb_ctlr->isp_proc, ESP_ERR_INVALID_ARG, TAG, "invalid argument: null pointer");
     ESP_RETURN_ON_FALSE(awb_ctlr->fsm == ISP_FSM_ENABLE, ESP_ERR_INVALID_STATE, TAG, "controller isn't in enable state");
+    xSemaphoreTake(awb_ctlr->stat_lock, 0);
+    awb_ctlr->fsm = ISP_FSM_INIT;
 
     isp_ll_enable_intr(awb_ctlr->isp_proc->hal.hw, ISP_LL_EVENT_AWB_MASK, false);
     isp_ll_awb_clk_enable(awb_ctlr->isp_proc->hal.hw, false);
     esp_intr_disable(awb_ctlr->intr_handle);
-    awb_ctlr->fsm = ISP_FSM_INIT;
-    xSemaphoreTake(awb_ctlr->stat_lock, 0);
 
     return ESP_OK;
 }
 
 esp_err_t esp_isp_awb_controller_get_oneshot_statistics(isp_awb_ctlr_t awb_ctlr, int timeout_ms, isp_awb_stat_result_t *out_res)
 {
-    ESP_RETURN_ON_FALSE_ISR(awb_ctlr && (out_res || timeout_ms == 0), ESP_ERR_INVALID_ARG, TAG, "invalid argument: null pointer");
-    ESP_RETURN_ON_FALSE_ISR(awb_ctlr->fsm == ISP_FSM_ENABLE, ESP_ERR_INVALID_STATE, TAG, "controller isn't in enable state");
+    ESP_RETURN_ON_FALSE(awb_ctlr && (out_res || timeout_ms == 0), ESP_ERR_INVALID_ARG, TAG, "invalid argument: null pointer");
+    esp_err_t ret = ESP_OK;
     TickType_t ticks = timeout_ms < 0 ? portMAX_DELAY : pdMS_TO_TICKS(timeout_ms);
-
     xSemaphoreTake(awb_ctlr->stat_lock, ticks);
+    ESP_GOTO_ON_FALSE(awb_ctlr->fsm == ISP_FSM_ENABLE, ESP_ERR_INVALID_STATE, err, TAG, "controller isn't in enable state");
     // Update state to avoid race condition
     awb_ctlr->fsm = ISP_FSM_START;
-    esp_err_t ret = ESP_OK;
     // Reset the queue in case receiving the legacy data in the queue
     xQueueReset(awb_ctlr->evt_que);
     // Start the AWB white patch statistics and waiting it done
@@ -187,30 +190,34 @@ esp_err_t esp_isp_awb_controller_get_oneshot_statistics(isp_awb_ctlr_t awb_ctlr,
     // Stop the AWB white patch statistics
     isp_ll_awb_enable(awb_ctlr->isp_proc->hal.hw, false);
     awb_ctlr->fsm = ISP_FSM_ENABLE;
+err:
     xSemaphoreGive(awb_ctlr->stat_lock);
-
     return ret;
 }
 
 esp_err_t esp_isp_awb_controller_start_continuous_statistics(isp_awb_ctlr_t awb_ctlr)
 {
-    ESP_RETURN_ON_FALSE_ISR(awb_ctlr, ESP_ERR_INVALID_ARG, TAG, "invalid argument: null pointer");
-    ESP_RETURN_ON_FALSE_ISR(awb_ctlr->fsm == ISP_FSM_ENABLE, ESP_ERR_INVALID_STATE, TAG, "controller isn't in enable state");
-
+    ESP_RETURN_ON_FALSE(awb_ctlr, ESP_ERR_INVALID_ARG, TAG, "invalid argument: null pointer");
+    ESP_RETURN_ON_FALSE(awb_ctlr->cbs.on_statistics_done, ESP_ERR_INVALID_STATE, TAG, "invalid state: on_statistics_done callback not registered");
+    esp_err_t ret = ESP_OK;
     if (xSemaphoreTake(awb_ctlr->stat_lock, 0) == pdFALSE) {
         ESP_LOGW(TAG, "statistics lock is not acquired, controller is busy");
         return ESP_ERR_INVALID_STATE;
     }
+    ESP_GOTO_ON_FALSE(awb_ctlr->fsm == ISP_FSM_ENABLE, ESP_ERR_INVALID_STATE, err, TAG, "controller isn't in enable state");
     awb_ctlr->fsm = ISP_FSM_START;
     isp_ll_awb_enable(awb_ctlr->isp_proc->hal.hw, true);
 
-    return ESP_OK;
+    return ret;
+err:
+    xSemaphoreGive(awb_ctlr->stat_lock);
+    return ret;
 }
 
 esp_err_t esp_isp_awb_controller_stop_continuous_statistics(isp_awb_ctlr_t awb_ctlr)
 {
-    ESP_RETURN_ON_FALSE_ISR(awb_ctlr, ESP_ERR_INVALID_ARG, TAG, "invalid argument: null pointer");
-    ESP_RETURN_ON_FALSE_ISR(awb_ctlr->fsm == ISP_FSM_START, ESP_ERR_INVALID_STATE, TAG, "controller isn't in continuous state");
+    ESP_RETURN_ON_FALSE(awb_ctlr, ESP_ERR_INVALID_ARG, TAG, "invalid argument: null pointer");
+    ESP_RETURN_ON_FALSE(awb_ctlr->fsm == ISP_FSM_START, ESP_ERR_INVALID_STATE, TAG, "controller isn't in continuous state");
 
     isp_ll_awb_enable(awb_ctlr->isp_proc->hal.hw, false);
     awb_ctlr->fsm = ISP_FSM_ENABLE;
@@ -236,7 +243,7 @@ static void IRAM_ATTR s_isp_awb_default_isr(void *arg)
         // Get the statistics result
         esp_isp_awb_evt_data_t edata = {
             .awb_result = {
-                .white_patch_num = isp_ll_awb_get_white_patcherence_cnt(proc->hal.hw),
+                .white_patch_num = isp_ll_awb_get_white_patch_cnt(proc->hal.hw),
                 .sum_r = isp_ll_awb_get_accumulated_r_value(proc->hal.hw),
                 .sum_g = isp_ll_awb_get_accumulated_g_value(proc->hal.hw),
                 .sum_b = isp_ll_awb_get_accumulated_b_value(proc->hal.hw),
diff --git a/components/esp_driver_isp/test_apps/isp/main/test_isp_driver.c b/components/esp_driver_isp/test_apps/isp/main/test_isp_driver.c
@@ -57,6 +57,15 @@ TEST_CASE("ISP AF controller exhausted allocation", "[isp]")
     TEST_ESP_OK(esp_isp_del_processor(isp_proc));
 }
 
+static bool test_isp_awb_default_on_statistics_done_cb(isp_awb_ctlr_t awb_ctlr, const esp_isp_awb_evt_data_t *edata, void *user_data)
+{
+    (void) awb_ctlr;
+    (void) edata;
+    (void) user_data;
+    // Do nothing
+    return false;
+}
+
 TEST_CASE("ISP AWB driver basic function", "[isp]")
 {
     esp_isp_processor_cfg_t isp_config = {
@@ -88,6 +97,11 @@ TEST_CASE("ISP AWB driver basic function", "[isp]")
     isp_awb_stat_result_t stat_res = {};
     /* Create the awb controller */
     TEST_ESP_OK(esp_isp_new_awb_controller(isp_proc, &awb_config, &awb_ctlr));
+    /* Register AWB callback */
+    esp_isp_awb_cbs_t awb_cb = {
+        .on_statistics_done = test_isp_awb_default_on_statistics_done_cb,
+    };
+    TEST_ESP_OK(esp_isp_awb_register_event_callbacks(awb_ctlr, &awb_cb, NULL));
     /* Enabled the awb controller */
     TEST_ESP_OK(esp_isp_awb_controller_enable(awb_ctlr));
     /* Start continuous AWB statistics */
diff --git a/components/hal/esp32p4/include/hal/isp_ll.h b/components/hal/esp32p4/include/hal/isp_ll.h
@@ -1123,7 +1123,7 @@ static inline void isp_ll_awb_set_bg_ratio_range(isp_dev_t *hw, isp_ll_awb_rgb_r
  *      - white patch count
  */
 __attribute__((always_inline))
-static inline uint32_t isp_ll_awb_get_white_patcherence_cnt(isp_dev_t *hw)
+static inline uint32_t isp_ll_awb_get_white_patch_cnt(isp_dev_t *hw)
 {
     return hw->awb0_white_cnt.awb0_white_cnt;
 }
diff --git a/components/hal/include/hal/isp_hal.h b/components/hal/include/hal/isp_hal.h
@@ -88,21 +88,6 @@ uint32_t isp_hal_check_clear_intr_event(const isp_hal_context_t *hal, uint32_t m
  */
 void isp_hal_bf_config(isp_hal_context_t *hal, isp_hal_bf_cfg_t *config);
 
-/*---------------------------------------------------------------
-                      Color Correction Matrix
----------------------------------------------------------------*/
-/**
- * @brief Set Color Correction Matrix
- *
- * @param[in] hal           Context of the HAL layer
- * @param[in] saturation    Whether to enable saturation when float data overflow
- * @param[in] flt_matrix    3x3 RGB correction matrix
- * @return
- *      - true      Set success
- *      - false     Invalid are
- */
-bool isp_hal_ccm_set_matrix(const isp_hal_context_t *hal, bool saturation, const float flt_matrix[3][3]);
-
 /*---------------------------------------------------------------
                             AWB
 ---------------------------------------------------------------*/
diff --git a/components/hal/include/hal/isp_types.h b/components/hal/include/hal/isp_types.h
@@ -82,12 +82,14 @@ typedef enum {
 #endif
 
 /**
- * @brief ISP AF result
+ * @brief ISP AF window
  */
 typedef struct {
-    int definition[ISP_AF_WINDOW_NUM];    ///< Definition, it refers how clear and sharp an image is
-    int luminance[ISP_AF_WINDOW_NUM];     ///< Luminance, it refers how luminant an image is
-} isp_af_result_t;
+    uint32_t top_left_x;        ///< Top left x axis value
+    uint32_t top_left_y;        ///< Top left y axis value
+    uint32_t bottom_right_x;    ///< Bottom right x axis value
+    uint32_t bottom_right_y;    ///< Bottom right y axis value
+} isp_af_window_t;
 
 /*---------------------------------------------------------------
                       BF
diff --git a/components/hal/isp_hal.c b/components/hal/isp_hal.c
@@ -16,8 +16,6 @@
 #include "hal/isp_types.h"
 #include "hal/hal_utils.h"
 
-#include "esp_rom_sys.h"
-
 /**
  * ISP HAL layer
  */
diff --git a/docs/en/api-reference/peripherals/isp.rst b/docs/en/api-reference/peripherals/isp.rst

Original file line number	Diff line number	Diff line change
`@@ -178,7 +178,7 @@ typedef struct {`
`178`	`178`	`/**`
`179`	`179`	`* @brief Prototype of ISP AF Env detector event callback`
`180`	`180`	`*`
`181`		`- * @param[in] af_ctrlr ISP AF controller handle`
	`181`	`+ * @param[in] af_ctrlr ISP AF controller handle`
`182`	`182`	`* @param[in] edata ISP AF Env detector event data`
`183`	`183`	* @param[in] user_data User registered context, registered when in `esp_isp_af_env_detector_register_event_callbacks()`
`184`	`184`	`*`
Original file line number	Diff line number	Diff line change
`@@ -1123,7 +1123,7 @@ static inline void isp_ll_awb_set_bg_ratio_range(isp_dev_t *hw, isp_ll_awb_rgb_r`
`1123`	`1123`	`* - white patch count`
`1124`	`1124`	`*/`
`1125`	`1125`	`__attribute__((always_inline))`
`1126`		`-static inline uint32_t isp_ll_awb_get_white_patcherence_cnt(isp_dev_t *hw)`
	`1126`	`+static inline uint32_t isp_ll_awb_get_white_patch_cnt(isp_dev_t *hw)`
`1127`	`1127`	`{`
`1128`	`1128`	`return hw->awb0_white_cnt.awb0_white_cnt;`
`1129`	`1129`	`}`