Hi Heiko, On Fri, Jan 8, 2021 at 5:48 AM Heiko Stuebner <heiko.stuebner@xxxxxxxxxxxxxxxxxxxxx> wrote: > > Hi, > > the rkisp driver in the mainline Linux kernel moved out of staging with > 5.11-rc1, so the uapi will be fixed after 5.11 proper is released. > > The rkisp driver currently only supports the rk3399 and while working > on porting the support for rk3326/px30 I noticed discrepancies. > > Hence it would be somewhat urgent to clarify this, as later it will get > really cumbersome. > > ---- > > The rkisp on the px30 (v12) has some changes compared to the rk3399 (v10). > Some sub-blocks moved around or seem to have been replaced with newer > variants and the gist of changes can be seen in [0] with the important > part being the uapi changes [1] and those values also exist in mainline. > > > See functions in that patch: > - isp_goc_config_v12() > - rkisp1_stats_get_aec_meas_v12() > - rkisp1_stats_get_hst_meas_v12() > > Looking at the code, the register locations are different, for gammas and > the histogram the actual amount of raw registers is the same, while the > "aec" seems to use 25 registers on V10 while 21 registers on V12. Though > their content gets split into multiple values in that v12 variant. > > > As somehow expected the whole thing is pretty undocumented and I > have no clue what these "bins" or "gammas" mean and why the amount of > entries now differs and how this relates to userspace at all. > > Also looking through libcamera as the one open user of the driver, > the whole rkisp1_cif_isp_isp_other_cfg (containing the gamma config) > as well as the rkisp1_cif_isp_stat struct (for ae and histogram) > don't seem to be used so far. FWIW, another open source user of the driver is the Rockchip HAL for Chrome OS. [1] The link points to an unmerged version that is updated for the upstream driver, which we want to transition to, as per the stack of patches at [2]. For example, RKISP1_CIF_ISP_HIST_BIN_N_MAX (which CIFISP_HIST_BIN_N_MAX was renamed to) seems to be referenced in [3]. [1] https://chromium.googlesource.com/chromiumos/platform2/+/5c055d6ae727e347c1a9fd8acad061e427b1e4ff/camera/hal/rockchip/ [2] https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2521601/ [3] https://chromium.googlesource.com/chromiumos/platform2/+/5c055d6ae727e347c1a9fd8acad061e427b1e4ff/camera/hal/rockchip/psl/rkisp1/workers/StatisticsWorker.cpp#220 Best regards, Tomasz > > Hence I also added some Rockchip people in the hope of getting > a bit of clarification ;-) . > > > Ideas on how to proceed? > > Thanks > Heiko > > > [0] https://github.com/rockchip-linux/kernel/commit/2ff670508e8fdfefd67318e885effb8cee4a0f4c > [1] > diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h > index b471f01a8459..fbeb6b5dba03 100644 > --- a/include/uapi/linux/rkisp1-config.h > +++ b/include/uapi/linux/rkisp1-config.h > @@ -32,8 +32,8 @@ > #define CIFISP_CTK_COEFF_MAX 0x100 > #define CIFISP_CTK_OFFSET_MAX 0x800 > > -#define CIFISP_AE_MEAN_MAX 25 > -#define CIFISP_HIST_BIN_N_MAX 16 > +#define CIFISP_AE_MEAN_MAX 81 > +#define CIFISP_HIST_BIN_N_MAX 32 > #define CIFISP_AFM_MAX_WINDOWS 3 > #define CIFISP_DEGAMMA_CURVE_SIZE 17 > > @@ -69,7 +69,7 @@ > * Gamma out > */ > /* Maximum number of color samples supported */ > -#define CIFISP_GAMMA_OUT_MAX_SAMPLES 17 > +#define CIFISP_GAMMA_OUT_MAX_SAMPLES 34 > > >