Skip to contentSkip to conversation

media: rkisp1: correct histogram window size

PATCH In review Krzysztof Hałasa 2 participants 2 messages May 23, 2025

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 media: rkisp1: correct histogram window size

Without the patch (i.MX8MP, all-white RGGB-12 full HD input from the sensor, YUV NV12 output from ISP, full range, histogram Y mode). HIST_STEPSIZE = 3 (lowest permitted), all weights are set to 1:

isp_hist_h_size: 383 (= 1920 / 5 - 1)
isp_hist_v_size: 215 (= 1080 / 5 - 1) histogram_measurement_result[16]: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 229401

Apparently the histogram is missing the last column (3-pixel wide, though only single pixels count) and the last (same idea) row of the input image: 1917 * 1077 / 3 / 3 = 229401.

E.g. without the patch, the pixels counted in lines are:
0, 3, ... 1914 (which makes 1917/3), and patched:
0, 3, ... 1914, 1917 (which makes 1920/3).
The same is true for rows.

With the patch applied:
isp_hist_h_size: 384 (= 1920 / 5)
isp_hist_v_size: 216 (= 1080 / 5) histogram_measurement_result[16]: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 230400

1920 * 1080 / 3 / 3 = 230400

drivers/media/platform/rockchip/rkisp1/rkisp1-params.c +2 -2
index b28f4140c8a3..ca9b3e711e5f 100644
@@ -819,8 +819,8 @@ static void rkisp1_hst_config_v10(struct rkisp1_params *params,
 		     arg->meas_window.v_offs);
 
 	block_hsize = arg->meas_window.h_size /
-		      RKISP1_CIF_ISP_HIST_COLUMN_NUM_V10 - 1;
-	block_vsize = arg->meas_window.v_size / RKISP1_CIF_ISP_HIST_ROW_NUM_V10 - 1;
+		      RKISP1_CIF_ISP_HIST_COLUMN_NUM_V10;
+	block_vsize = arg->meas_window.v_size / RKISP1_CIF_ISP_HIST_ROW_NUM_V10;
 
 	rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_HIST_H_SIZE_V10,
 		     block_hsize);
Signature · 5 lines
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa
Reviewed-by on v1 1/1
#1 lore raw

Hi Krzysztof,

Thanks for the new version.

My bad, I should've mentioned in the review of the previous version; I think you should have a paragraph in the beginning briefly explaining the problem in english.

"The histogram window dimensions are unnecessarily decremented. This can be seen in the following example, where when all pixels are of maximum intensity there are pixels that we expect to be counted but are missing from the highest bin. Fix this by removing the unnecessary decrement."

I think would be good enough.

Quoting Krzysztof Hałasa (2025-05-23 18:10:47)

Quoted 8 lines

Without the patch (i.MX8MP, all-white RGGB-12 full HD input from the sensor, YUV NV12 output from ISP, full range, histogram Y mode). HIST_STEPSIZE = 3 (lowest permitted), all weights are set to 1:

isp_hist_h_size: 383 (= 1920 / 5 - 1)
isp_hist_v_size: 215 (= 1080 / 5 - 1) histogram_measurement_result[16]: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 229401

Quoted 8 lines

Apparently the histogram is missing the last column (3-pixel wide, though only single pixels count) and the last (same idea) row of the input image: 1917 * 1077 / 3 / 3 = 229401.

E.g. without the patch, the pixels counted in lines are:
0, 3, ... 1914 (which makes 1917/3), and patched:
0, 3, ... 1914, 1917 (which makes 1920/3).
The same is true for rows.

I like this clarification.

With the above paragraph added,

(I suppose if you approve it then it could simply be added when it's applied and you wouldn't need to send a new version, but I don't have permissions for that so uh...)

Quoted 32 lines

With the patch applied:
isp_hist_h_size: 384 (= 1920 / 5)
isp_hist_v_size: 216 (= 1080 / 5) histogram_measurement_result[16]: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 230400

1920 * 1080 / 3 / 3 = 230400

drivers/media/platform/rockchip/rkisp1/rkisp1-params.c +2 -2
index b28f4140c8a3..ca9b3e711e5f 100644
@@ -819,8 +819,8 @@ static void rkisp1_hst_config_v10(struct rkisp1_params *params,
                     arg->meas_window.v_offs);
 
        block_hsize = arg->meas_window.h_size /
-                     RKISP1_CIF_ISP_HIST_COLUMN_NUM_V10 - 1;
-       block_vsize = arg->meas_window.v_size / RKISP1_CIF_ISP_HIST_ROW_NUM_V10 - 1;
+                     RKISP1_CIF_ISP_HIST_COLUMN_NUM_V10;
+       block_vsize = arg->meas_window.v_size / RKISP1_CIF_ISP_HIST_ROW_NUM_V10;
 
        rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_HIST_H_SIZE_V10,
                     block_hsize);
Signature · 6 lines
-- 
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa