Re: [v8 08/14] media: rppx1: bls: Add support for black level compensation

Jacopo Mondi
2026-05-06T15:25:05Z

Hi Niklas

On Mon, May 04, 2026 at 03:05:50AM +0200, Niklas Söderlund wrote:
> Extend the RPPX1 driver to allow setting the black level measurement
> and gain configuration and consuming the resulting statistics. It uses
> the RPPX1 framework for parameters and its writer abstraction to allow
> the user to control how, and when, configuration is applied to the
> RPPX1.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Co-developed-by: Jai Luthra <jai.luthra+renesas@ideasonboard.com>
> Signed-off-by: Jai Luthra <jai.luthra+renesas@ideasonboard.com>
> ---
> .../platform/dreamchip/rppx1/rpp_module.h | 2 +
> .../platform/dreamchip/rppx1/rpp_params.c | 5 +
> .../platform/dreamchip/rppx1/rpp_stats.c | 8 ++
> .../platform/dreamchip/rppx1/rppx1_bls.c | 121 ++++++++++++++++
> .../uapi/linux/media/dreamchip/rppx1-config.h | 131 +++++++++++++++++-
> 5 files changed, 265 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_module.h b/drivers/media/platform/dreamchip/rppx1/rpp_module.h
> index 5725243d0119..9c761448717b 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rpp_module.h
> +++ b/drivers/media/platform/dreamchip/rppx1/rpp_module.h
> @@ -85,6 +85,7 @@ void rpp_module_clrset(struct rpp_module *mod, u32 offset, u32 mask, u32 value);
>
> union rppx1_params_block {
> struct v4l2_isp_params_block_header header;
> + struct rppx1_bls_params bls;
> struct rppx1_awbg_params awbg;
> struct rppx1_hist_params hist;
> struct rppx1_exm_params exm;
> @@ -93,6 +94,7 @@ union rppx1_params_block {
>
> union rppx1_stats_block {
> struct v4l2_isp_params_block_header header;
> + struct rppx1_bls_stats bls;
> struct rppx1_hist_stats hist;
> struct rppx1_exm_stats exm;
> struct rppx1_wbmeas_stats wbmeas;
> diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> index 6472bec6fba3..7b006c68381b 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> +++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> @@ -16,6 +16,8 @@
>
> static const struct v4l2_isp_block_type_info
> rppx1_ext_params_blocks_info[] = {
> + RPPX1_PARAMS_BLOCK_INFO(BLS_PRE1, bls),
> + RPPX1_PARAMS_BLOCK_INFO(BLS_PRE2, bls),
> RPPX1_PARAMS_BLOCK_INFO(AWBG_PRE1, awbg),
> RPPX1_PARAMS_BLOCK_INFO(AWBG_PRE2, awbg),
> RPPX1_PARAMS_BLOCK_INFO(HIST_PRE1, hist),
> @@ -58,6 +60,9 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb, size_t max_size,
> block_offset += block->header.size;
>
> switch (block->header.type) {
> + case RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE1:
> + module = &rpp->pre1.bls;
> + break;
> case RPPX1_PARAMS_BLOCK_TYPE_AWBG_PRE1:
> module = &rpp->pre1.awbg;
> break;
> diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_stats.c b/drivers/media/platform/dreamchip/rppx1/rpp_stats.c
> index b265e858cfd1..c2c0fc109e90 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rpp_stats.c
> +++ b/drivers/media/platform/dreamchip/rppx1/rpp_stats.c
> @@ -16,6 +16,7 @@
>
> static const struct v4l2_isp_block_type_info
> rppx1_stats_blocks_info[] = {
> + RPPX1_STATS_BLOCK_INFO(BLS_PRE1, bls),
> RPPX1_STATS_BLOCK_INFO(HIST_POST, hist),
> RPPX1_STATS_BLOCK_INFO(EXM_PRE1, exm),
> RPPX1_STATS_BLOCK_INFO(WBMEAS_POST, wbmeas),
> @@ -51,6 +52,13 @@ void rppx1_stats_fill_isr(struct rppx1 *rpp, u32 isc, void *buf)
> return;
>
> rpp_module_call(&rpp->pre1.exm, fill_stats, block);
> +
> + block = rppx1_init_stats_block(rpp, stats,
> + RPPX1_STATS_BLOCK_TYPE_BLS_PRE1);
> + if (!block)
> + return;
> +
> + rpp_module_call(&rpp->pre1.bls, fill_stats, block);
> }
>
> if (isc & RPPX1_IRQ_ID_POST_AWB_MEAS) {
> diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> index de7008befd8e..a3cb03fd97d0 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> +++ b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> @@ -5,6 +5,7 @@
> */
>
> #include "rpp_module.h"
> +#include "rppx1.h"
>
> #define BLS_VERSION_REG 0x0000
>
> @@ -31,6 +32,8 @@
> #define BLS_B_MEASURED_REG 0x0040
> #define BLS_C_MEASURED_REG 0x0044
> #define BLS_D_MEASURED_REG 0x0048
> +#define BLS_PRE1_FIXED_MASK GENMASK(24, 0)
> +#define BLS_PRE2_FIXED_MASK GENMASK(12, 0)
>
> static int rppx1_bls_probe(struct rpp_module *mod)
> {
> @@ -54,6 +57,124 @@ static int rppx1_bls_probe(struct rpp_module *mod)
> return 0;
> }
>
> +static void
> +rppx1_bls_swap_regs(struct rpp_module *mod, const u32 input[4], u32 output[4])
> +{
> + static const unsigned int swap[4][4] = {
> + [RPP_RGGB] = { 0, 1, 2, 3 },
> + [RPP_GRBG] = { 1, 0, 3, 2 },
> + [RPP_GBRG] = { 2, 3, 0, 1 },
> + [RPP_BGGR] = { 3, 2, 1, 0 },
> + };
> +
> + /* Swap to pattern used in our path, PRE1 or PRE2. */
> + struct rpp_module *acq = mod == &mod->rpp->pre1.bls ?
> + &mod->rpp->pre1.acq : &mod->rpp->pre2.bls;
> + enum rpp_raw_pattern pattern = acq->info.acq.raw_pattern;
> +
> + for (unsigned int i = 0; i < 4; ++i)
> + output[i] = input[swap[pattern][i]];
> +}
> +
> +static int
> +rppx1_bls_fill_params(struct rpp_module *mod,
> + const union rppx1_params_block *block,
> + rppx1_reg_write write, void *priv)
> +{
> + const struct rppx1_bls_params *cfg = &block->bls;
> +
> + /* If the modules is disabled, simply bypass it. */
> + if (cfg->header.flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) {
> + write(priv, mod->base + BLS_CTRL_REG, 0);
> + return 0;
> + }
> +
> + u32 ctrl = BLS_CTRL_BLS_EN;
> +
> + if (cfg->mode == RPPX1_BLS_MODE_FIXED) {
> + static const u32 regs[] = {
> + BLS_A_FIXED_REG,
> + BLS_B_FIXED_REG,
> + BLS_C_FIXED_REG,
> + BLS_D_FIXED_REG,
> + };
> + u32 swapped[4];
> +
> + rppx1_bls_swap_regs(mod, regs, swapped);
> +
> + /*
> + * The PRE1 pipe fixed values are 24-bits + 1 sign bit, while
> + * the PRE2 pipe values are 12-bits + 1 sign bit.
> + */
> + u32 mask;
> +
> + switch (cfg->header.type) {
> + case RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE1:
> + mask = BLS_PRE1_FIXED_MASK;
> + break;
> + case RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE2:
> + mask = BLS_PRE2_FIXED_MASK;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + write(priv, mod->base + swapped[0], cfg->fixed.a & mask);
> + write(priv, mod->base + swapped[1], cfg->fixed.b & mask);
> + write(priv, mod->base + swapped[2], cfg->fixed.c & mask);
> + write(priv, mod->base + swapped[3], cfg->fixed.d & mask);
> + } else {
> + write(priv, mod->base + BLS_SAMPLES_REG, cfg->samples);
> +
> + if (cfg->en_windows & RPPX1_BLS_WIN_EN_WIN1) {
> + write(priv, mod->base + BLS_H1_START_REG, cfg->window1.h_offs);
> + write(priv, mod->base + BLS_H1_STOP_REG, cfg->window1.h_size);
> + write(priv, mod->base + BLS_V1_START_REG, cfg->window1.v_offs);
> + write(priv, mod->base + BLS_V1_STOP_REG, cfg->window1.v_size);
> + ctrl |= BLS_CTRL_BLS_WIN1;
> + }
> +
> + if (cfg->en_windows & RPPX1_BLS_WIN_EN_WIN2) {
> + write(priv, mod->base + BLS_H2_START_REG, cfg->window2.h_offs);
> + write(priv, mod->base + BLS_H2_STOP_REG, cfg->window2.h_size);
> + write(priv, mod->base + BLS_V2_START_REG, cfg->window2.v_offs);
> + write(priv, mod->base + BLS_V2_STOP_REG, cfg->window2.v_size);
> + ctrl |= BLS_CTRL_BLS_WIN2;
> + }
> +
> + ctrl |= BLS_CTRL_BLS_MODE_MEASURED;
> + }
> +
> + write(priv, mod->base + BLS_CTRL_REG, ctrl);
> +
> + return 0;
> +}
> +
> +static int rppx1_bls_fill_stats(struct rpp_module *mod,
> + union rppx1_stats_block *block)
> +{

I had left out bls_stats for two reasons:
- they need a properly configured sensor to produce optically black
pixels
- I don't think they should be reported in stats for each frame, after
all their measurement should be fixed ?

All in all this seems like a feature to be used for calibration. I'm
not saying we should prevent it from being used, but maybe reporting
it through stats is not the best way ?

> + struct rppx1_bls_stats *stats = &block->bls;
> +
> + static const u32 regs[] = {
> + BLS_A_MEASURED_REG,
> + BLS_B_MEASURED_REG,
> + BLS_C_MEASURED_REG,
> + BLS_D_MEASURED_REG,
> + };
> + u32 swapped[4];
> +
> + rppx1_bls_swap_regs(mod, regs, swapped);
> +
> + stats->meas_r = rpp_module_read(mod, swapped[0]);
> + stats->meas_gr = rpp_module_read(mod, swapped[1]);
> + stats->meas_gb = rpp_module_read(mod, swapped[2]);
> + stats->meas_b = rpp_module_read(mod, swapped[3]);

For params I used 'a' 'b' 'c' and 'd' to match the registers. True
that with rppx1_bls_swap_regs() the a,b,c and d channels gets mapped
to the color ones. Should we use color channels for params as well ?

> +
> + return 0;
> +}
> +
> const struct rpp_module_ops rppx1_bls_ops = {
> .probe = rppx1_bls_probe,
> + .fill_params = rppx1_bls_fill_params,
> + .fill_stats = rppx1_bls_fill_stats,
> };
> diff --git a/include/uapi/linux/media/dreamchip/rppx1-config.h b/include/uapi/linux/media/dreamchip/rppx1-config.h
> index 909a10935772..b181ef08d093 100644
> --- a/include/uapi/linux/media/dreamchip/rppx1-config.h
> +++ b/include/uapi/linux/media/dreamchip/rppx1-config.h
> @@ -84,6 +84,8 @@ enum rppx1_meas_chan {
> * @RPPX1_PARAMS_BLOCK_TYPE_HIST_PRE1: PRE1 pipe Histogram Measurement
> * @RPPX1_PARAMS_BLOCK_TYPE_HIST_PRE2: PRE2 pipe Histogram Measurement
> * @RPPX1_PARAMS_BLOCK_TYPE_HIST_POST: POST pipe Histogram Measurement
> + * @RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE1: PRE1 pipe Black Level Subtraction
> + * @RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE2: PRE2 pipe Black Level Subtraction
> */
> enum rppx1_params_block_type {
> RPPX1_PARAMS_BLOCK_TYPE_WBMEAS_POST,
> @@ -95,6 +97,8 @@ enum rppx1_params_block_type {
> RPPX1_PARAMS_BLOCK_TYPE_HIST_PRE1,
> RPPX1_PARAMS_BLOCK_TYPE_HIST_PRE2,
> RPPX1_PARAMS_BLOCK_TYPE_HIST_POST,
> + RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE1,
> + RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE2,
> };
>
> /**
> @@ -319,6 +323,103 @@ struct rppx1_hist_params {
> __u8 sample_shift;
> };
>
> +/**
> + * struct rppx1_bls_fixed - BLS fixed subtraction values
> + *
> + * Fixed black level values subtracted from sensor data per Bayer channel.
> + * Negative values result in addition.
> + *
> + * The PRE1 pipe BLS module operates on a 24-bits input data and fixed black
> + * levels are stored as a signed 2's complement representation ranging from
> + * -2^24 to 2^24-1.
> + *
> + * The PRE2 pipe BLS module operates on a 12-bits input data and fixed black
> + * levels are stored as a signed 2's complement representation ranging from
> + * -2^12 to 2^12-1.
> + *
> + * Userspace is expected to provide fixed black level values with a bit-depth
> + * matching the one of pipe in use.
> + *
> + * These subtraction values are matched with the sensor native Bayer components
> + * ordering according to the cropping configuration on the input port.
> + *
> + * @a: subtraction value for channel A
> + * @b: subtraction value for channel B
> + * @c: subtraction value for channel C
> + * @d: subtraction value for channel D
> + */
> +struct rppx1_bls_fixed {
> + __u32 a;
> + __u32 b;
> + __u32 c;
> + __u32 d;
> +};
> +
> +/**
> + * enum rppx1_bls_mode - BLS subtraction mode
> + *
> + * Select if subtracted black level come from fixed or measured values.
> + *
> + * @RPPX1_BLS_MODE_FIXED: subtract fixed values
> + * @RPPX1_BLS_MODE_MEAS: subtract measured values
> + */
> +enum rppx1_bls_mode {
> + RPPX1_BLS_MODE_FIXED,
> + RPPX1_BLS_MODE_MEAS,
> +};
> +
> +/**
> + * enum rppx1_bls_win_en: BLS measurement configuration
> + *
> + * Select the measurement window to use for measured black level values.
> + *
> + * @RPPX1_BLS_WIN_EN_OFF: disable measurement
> + * @RPPX1_BLS_WIN_EN_WIN1: Enable measurement from window 1
> + * @RPPX1_BLS_WIN_EN_WIN2: enable measurement from window 2
> + * @RPPX1_BLS_WIN_EN_WIN12: enable measurement from window 1 and window 2
> + */
> +enum rppx1_bls_win_en {
> + RPPX1_BLS_WIN_EN_OFF,
> + RPPX1_BLS_WIN_EN_WIN1,
> + RPPX1_BLS_WIN_EN_WIN2,
> + RPPX1_BLS_WIN_EN_WIN12,
> +};
> +
> +/**
> + * struct rppx1_bls_params - RPP-X1 Black Level Subtraction Module
> + *
> + * The RPP-X1 Black Level Subtraction module is available on the PRE1 and PRE2
> + * pre-fusion pipes. Userspace selects which pipe to operate by setting the
> + * @header.type field to RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE1 or
> + * RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE2.
> + *
> + * The BLS module operates on fixed or measured data according to the setting of
> + * the @mode field. When RPPX1_BLS_MODE_FIXED is used userspace shall provide
> + * the per-channel black levels in @fixed. When RPPX1_BLS_MODE_MEAS is used
> + * userspace shall configure the measurement windows @window1 and optionally
> + * @window2 to select the optically black pixels region in the input frame. The
> + * @samples fields controls how many measure samples are used for averaging the
> + * measured black levels.
> + *
> + * @header: block header (type = RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE1 or
> + * type == RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE2)
> + * @mode: BLS subtraction mode (see enum rppx1_bls_mode)
> + * @en_windows: BLS measurement mode (see rppx1_bls_win_en)
> + * @samples: log2 of the number of measured pixels per Bayer position
> + * @window1: BLS measurement window 1 (14 bits)
> + * @window2: BLS measurement window 2 (14 bits)
> + * @fixed: fixed subtraction values (see enum rppx1_bls_fixed)
> + */
> +struct rppx1_bls_params {
> + struct v4l2_isp_params_block_header header;
> + __u8 mode;
> + __u8 en_windows;
> + __u8 samples;
> + struct rppx1_window window1;
> + struct rppx1_window window2;
> + struct rppx1_bls_fixed fixed;
> +};
> +
> /**
> * RPPX1_PARAMS_MAX_SIZE - Maximum size of all RPP-X1 parameter blocks
> *
> @@ -334,7 +435,9 @@ struct rppx1_hist_params {
> sizeof(struct rppx1_exm_params) + \
> sizeof(struct rppx1_hist_params) + \
> sizeof(struct rppx1_hist_params) + \
> - sizeof(struct rppx1_hist_params))
> + sizeof(struct rppx1_hist_params) + \
> + sizeof(struct rppx1_bls_params) + \
> + sizeof(struct rppx1_bls_params))
>
> /* ---------------------------------------------------------------------------
> * Statistics Structures
> @@ -354,6 +457,8 @@ struct rppx1_hist_params {
> * @RPPX1_STATS_BLOCK_TYPE_HIST_PRE1: pre-fusion pipe1 histogram
> * @RPPX1_STATS_BLOCK_TYPE_HIST_PRE2: pre-fusion pipe2 histogram
> * @RPPX1_STATS_BLOCK_TYPE_HIST_POST: post-fusion histogram
> + * @RPPX1_STATS_BLOCK_TYPE_BLS_PRE1: pre-fusion pipe1 black level subtraction
> + * @RPPX1_STATS_BLOCK_TYPE_BLS_PRE2: pre-fusion pipe2 black level subtraction
> */
> enum rppx1_stats_block_type {
> RPPX1_STATS_BLOCK_TYPE_WBMEAS_POST,
> @@ -362,6 +467,8 @@ enum rppx1_stats_block_type {
> RPPX1_STATS_BLOCK_TYPE_HIST_PRE1,
> RPPX1_STATS_BLOCK_TYPE_HIST_PRE2,
> RPPX1_STATS_BLOCK_TYPE_HIST_POST,
> + RPPX1_STATS_BLOCK_TYPE_BLS_PRE1,
> + RPPX1_STATS_BLOCK_TYPE_BLS_PRE2,
> };
>
> /**
> @@ -412,6 +519,24 @@ struct rppx1_hist_stats {
> __u32 hist_bins[RPPX1_HIST_NUM_BINS];
> };
>
> +/**
> + * struct rppx1_bls_stats - Black level subtraction measurements

If we want to report stats, please add documentation with a similar
level of details as the one of rppx1_bls_params when it comes to the
bit depth of the measured data.

> + *
> + * @header: block header (type = RPPX1_STATS_BLOCK_TYPE_BLS_PRE1 or
> + * RPPX1_STATS_BLOCK_TYPE_BLS_PRE2)
> + * @meas_r: mean measured value for Bayer pattern R
> + * @meas_gr: mean measured value for Bayer pattern Gr
> + * @meas_gb: mean measured value for Bayer pattern Gb
> + * @meas_b: mean measured value for Bayer pattern B
> + */
> +struct rppx1_bls_stats {
> + struct v4l2_isp_block_header header;
> + __u32 meas_r;
> + __u32 meas_gr;
> + __u32 meas_gb;
> + __u32 meas_b;
> +};
> +
> /**
> * RPPX1_STATS_MAX_SIZE - Maximum size of all RPP-X1 statistics
> *
> @@ -424,6 +549,8 @@ struct rppx1_hist_stats {
> sizeof(struct rppx1_exm_stats) + \
> sizeof(struct rppx1_hist_stats) + \
> sizeof(struct rppx1_hist_stats) + \
> - sizeof(struct rppx1_hist_stats))
> + sizeof(struct rppx1_hist_stats) + \
> + sizeof(struct rppx1_bls_stats) + \
> + sizeof(struct rppx1_bls_stats))
>
> #endif /* __UAPI_RPP_X1_CONFIG_H */
> --
> 2.54.0
>
>