Am 16.01.21 um 00:52 schrieb Heiko Stübner:
Hi Dafna,
Am Freitag, 15. Januar 2021, 18:41:06 CET schrieb Dafna Hirschfeld:
Am 15.01.21 um 17:38 schrieb Heiko Stuebner:
From: Heiko Stuebner <heiko.stuebner@xxxxxxxxxxxxxxxxxxxxx>
Later variants of the rkisp1 block use more entries in some arrays:
RKISP1_CIF_ISP_AE_MEAN_MAX 25 -> 81
RKISP1_CIF_ISP_HIST_BIN_N_MAX 16 -> 32
RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES 17 -> 34
RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE 28 -> 81
I see you didn't change the value for that define.
In the below patch I find
@@ -103,7 +111,9 @@
* Histogram calculation
*/
/* Last 3 values unused. */
-#define RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE 28
+#define RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10 28
+#define RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12 81
+#define RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12
so I'm not sure I understand what you mean except this.
Opps, I somehow missed that.
But now since we have 81 entries, it makes no sense to
define it to 28 for V10 and documenting "Last 3 values unused" (see just above the definition).
We can set it just to 25, we have 56 (81-25) unused values anyway.
The usage of it is a bit more complicated.
It is used in function rkisp1_hst_config.
Yeah, though the for-loop iterates over 4*7 entry values, so stays
below the RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10 in all cases.
Actually the real number of weight values are 25 (5x5) for rk3399,
the last 3 are not used. I think that in order to support both
5x5 and 9x9 the code in rkisp1-params.c should change. I'll
send a patch fixing it.
If you look at my V12-patch [0] the weight handling is done different there
and from the registers, it looks like they exchanges that part of the isp.
[0] https://lore.kernel.org/linux-media/20210108193311.3423236-11-heiko@xxxxxxxxx/
void rkisp1_hst_config_v12() as a search term
[...]
Right, there is no need to change the relevant code in rkisp1-param.c when setting the
RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE to 25.
Thanks,
Dafna
@@ -862,8 +898,16 @@ struct rkisp1_cif_isp_af_stat {
*
* @hist_bins: measured bin counters
*
- * Measurement window divided into 16 sub-windows, set
- * with ISP_HIST_XXX
+ * Measurement window divided into 16 sub-windows for V10/V10
+ * and 32 sub-windows for V12/V13, set with ISP_HIST_XXX
It is actually not windows but histogram bins. Could you change it to:
"The histogram values divided into 16 bins for V10/V11 and 32 bins
for V12/V13. It is configured within the struct rkisp1_cif_isp_hst_config.
I've changed this like your suggestions and will give a bit of time for
the stuff above. But I guess I can send a v5 some time tomorrow?
Thanks for your input
Heiko
