[PATCH v4 3/3] media: rockchip: rkisp1: extend uapi array sizes
Dafna Hirschfeld
dafna.hirschfeld at collabora.com
Sat Jan 16 04:05:39 EST 2021
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
>
>
More information about the Linux-rockchip
mailing list