[PATCH v4 3/3] media: rockchip: rkisp1: extend uapi array sizes

[Dafna Hirschfeld]

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 at theobroma-systems.com>
>>>
>>> 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@sntech.de/
> 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
> 
>