Re: [PATCH v9 13/13] media: rppx1: lin: Add support for gamma sensor linearization

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Jacopo,

Thanks for your feedback.

On 2026-06-03 16:36:52 +0200, Jacopo Mondi wrote:
> Hi Niklas
> 
> On Sat, May 16, 2026 at 11:13:20PM +0200, Niklas Söderlund wrote:
> > From: Jai Luthra <jai.luthra@ideasonboard.com>
> >
> > Extend the RPPX1 driver to allow setting the gamma sensor linearization
> > configuration parameters. 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: Jai Luthra <jai.luthra@ideasonboard.com>
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> > * Changes since v8
> > - Move declaration of dx_lo and dx_hi to top of function.
> > ---
> >  .../platform/dreamchip/rppx1/rpp_module.h     |  1 +
> >  .../platform/dreamchip/rppx1/rpp_params.c     |  5 ++
> >  .../platform/dreamchip/rppx1/rppx1_lin.c      | 54 +++++++++++++++++++
> >  .../uapi/linux/media/dreamchip/rppx1-config.h | 48 ++++++++++++++++-
> >  4 files changed, 107 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_module.h b/drivers/media/platform/dreamchip/rppx1/rpp_module.h
> > index 948e26d7361d..072309d8662d 100644
> > --- a/drivers/media/platform/dreamchip/rppx1/rpp_module.h
> > +++ b/drivers/media/platform/dreamchip/rppx1/rpp_module.h
> > @@ -48,6 +48,7 @@ void rpp_module_clrset(struct rpp_module *mod, u32 offset, u32 mask, u32 value);
> >  union rppx1_params_block {
> >  	struct v4l2_isp_block_header header;
> >  	struct rppx1_bls_params bls;
> > +	struct rppx1_lin_params lin;
> >  	struct rppx1_lsc_params lsc;
> >  	struct rppx1_awbg_params awbg;
> >  	struct rppx1_ccor_params ccor;
> > diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> > index 86d445b52504..a75a27a8afd0 100644
> > --- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> > +++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> > @@ -19,6 +19,8 @@ static const struct v4l2_isp_params_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(LIN_PRE1, lin),
> > +	RPPX1_PARAMS_BLOCK_INFO(LIN_PRE2, lin),
> >  	RPPX1_PARAMS_BLOCK_INFO(LSC_PRE1, lsc),
> >  	RPPX1_PARAMS_BLOCK_INFO(LSC_PRE2, lsc),
> >  	RPPX1_PARAMS_BLOCK_INFO(AWBG_PRE1, awbg),
> > @@ -68,6 +70,9 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb, size_t max_size,
> >  		case RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE1:
> >  			module = &rpp->pre1.bls;
> >  			break;
> > +		case RPPX1_PARAMS_BLOCK_TYPE_LIN_PRE1:
> > +			module = &rpp->pre1.lin;
> > +			break;
> >  		case RPPX1_PARAMS_BLOCK_TYPE_LSC_PRE1:
> >  			module = &rpp->pre1.lsc;
> >  			break;
> > diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1_lin.c b/drivers/media/platform/dreamchip/rppx1/rppx1_lin.c
> > index 0a0e7ab2d1d6..04a92f24ab49 100644
> > --- a/drivers/media/platform/dreamchip/rppx1/rppx1_lin.c
> > +++ b/drivers/media/platform/dreamchip/rppx1/rppx1_lin.c
> > @@ -25,6 +25,11 @@
> >  #define LIN_B_Y_REG_NUM				17
> >  #define LIN_B_Y_REG(n)				(0x0098 + (4 * (n)))
> >
> > +#define LIN_PRE1_DEGAMMA_CURVE_MASK		GENMASK(23, 0)
> > +#define LIN_PRE1_SAMPLE_POINTS_MASK		GENMASK(3, 0)
> > +#define LIN_PRE2_DEGAMMA_CURVE_MASK		GENMASK(11, 0)
> > +#define LIN_PRE2_SAMPLE_POINTS_MASK		GENMASK(2, 0)
> > +
> >  static int rppx1_lin_probe(struct rpp_module *mod)
> >  {
> >  	/* Version check. */
> > @@ -53,7 +58,56 @@ static int rppx1_lin_start(struct rpp_module *mod,
> >  	return 0;
> >  }
> >
> > +static int rppx1_lin_fill_params(struct rpp_module *mod,
> > +				 const union rppx1_params_block *block,
> > +				 rppx1_reg_write write, void *priv)
> > +{
> > +	const struct rppx1_lin_params *cfg = &block->lin;
> > +	u8 sample_mask;
> > +	u32 dx_lo = 0;
> > +	u32 dx_hi = 0;
> > +	u32 mask;
> > +
> > +	if (cfg->header.flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) {
> > +		write(priv, mod->base + LIN_ENABLE_REG, 0);
> > +		return 0;
> > +	}
> > +
> > +	switch (cfg->header.type) {
> > +	case RPPX1_PARAMS_BLOCK_TYPE_LIN_PRE1:
> > +		mask = LIN_PRE1_DEGAMMA_CURVE_MASK;
> > +		sample_mask = LIN_PRE1_SAMPLE_POINTS_MASK;
> > +		break;
> > +	case RPPX1_PARAMS_BLOCK_TYPE_LIN_PRE2:
> > +		mask = LIN_PRE2_DEGAMMA_CURVE_MASK;
> > +		sample_mask = LIN_PRE2_SAMPLE_POINTS_MASK;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (unsigned int i = 0; i < 8; ++i) {
> > +		dx_lo |= (cfg->dx[i] & sample_mask) << 4 * i;
> > +		dx_hi |= (cfg->dx[i + 8] & sample_mask) << 4 * i;
> > +	}
> > +
> > +	write(priv, mod->base + LIN_DX_LO_REG, dx_lo);
> > +	write(priv, mod->base + LIN_DX_HI_REG, dx_hi);
> > +
> > +	for (unsigned int i = 0; i < RPPX1_LIN_DEGAMMA_CURVE_NUM; i++) {
> > +		write(priv, mod->base + LIN_R_Y_REG(i), cfg->curve_r[i] & mask);
> > +		write(priv, mod->base + LIN_G_Y_REG(i), cfg->curve_g[i] & mask);
> > +		write(priv, mod->base + LIN_B_Y_REG(i), cfg->curve_b[i] & mask);
> > +	}
> > +
> > +	if ((cfg->header.flags & V4L2_ISP_PARAMS_FL_BLOCK_ENABLE))
> 
> Why is this conditional ?
> 
> Do you think it could be enabled unconditionally ? the "disable" case
> is handled already above, isn't it ?

Wops, my bad. Yes it should be dropped.

> 
> 
> > +		write(priv, mod->base + LIN_ENABLE_REG, LIN_ENABLE_GAMMA_IN_EN);
> > +
> > +	return 0;
> > +}
> > +
> >  const struct rpp_module_ops rppx1_lin_ops = {
> >  	.probe = rppx1_lin_probe,
> >  	.start = rppx1_lin_start,
> > +	.fill_params = rppx1_lin_fill_params,
> >  };
> > diff --git a/include/uapi/linux/media/dreamchip/rppx1-config.h b/include/uapi/linux/media/dreamchip/rppx1-config.h
> > index 05faec2f68e3..72f3fd836f41 100644
> > --- a/include/uapi/linux/media/dreamchip/rppx1-config.h
> > +++ b/include/uapi/linux/media/dreamchip/rppx1-config.h
> > @@ -93,6 +93,8 @@ enum rppx1_meas_chan {
> >   * @RPPX1_PARAMS_BLOCK_TYPE_LSC_PRE2: PRE2 Lens Shading Correction
> >   * @RPPX1_PARAMS_BLOCK_TYPE_GA_HV: Human Vision Pipe Gamma Out Correction
> >   * @RPPX1_PARAMS_BLOCK_TYPE_GA_MV: Machine Vision Gamma Out Correction
> > + * @RPPX1_PARAMS_BLOCK_TYPE_LIN_PRE1: PRE1 pipe Linearization (Sensor De-gamma)
> > + * @RPPX1_PARAMS_BLOCK_TYPE_LIN_PRE2: PRE2 pipe Linearization (Sensor De-gamma)
> >   */
> >  enum rppx1_params_block_type {
> >  	RPPX1_PARAMS_BLOCK_TYPE_WBMEAS_POST,
> > @@ -111,6 +113,8 @@ enum rppx1_params_block_type {
> >  	RPPX1_PARAMS_BLOCK_TYPE_LSC_PRE2,
> >  	RPPX1_PARAMS_BLOCK_TYPE_GA_HV,
> >  	RPPX1_PARAMS_BLOCK_TYPE_GA_MV,
> > +	RPPX1_PARAMS_BLOCK_TYPE_LIN_PRE1,
> > +	RPPX1_PARAMS_BLOCK_TYPE_LIN_PRE2,
> >  };
> >
> >  /**
> > @@ -546,6 +550,46 @@ struct rppx1_ga_params {
> >  	__u32 gamma_y[RPPX1_GA_MAX_SAMPLES];
> >  };
> >
> > +/* Linearization (Sensor De-gamma) */
> > +#define RPPX1_LIN_SAMPLE_POINTS_NUM 16
> > +#define RPPX1_LIN_DEGAMMA_CURVE_NUM 17
> > +
> > +/**
> > + * struct rppx1_lin_params - Linearization (Sensor De-gamma) configuration
> > + *
> > + * The RPP-X1 linearization 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_LIN_PRE1 or
> > + * RPPX1_PARAMS_BLOCK_TYPE_LIN_PRE2.
> > + *
> > + * The LIN module applies the per-color channel de-gamma linearization curves
> > + * @curve_r, @curve_g and @curve_b defined on the input sampling points @dx.
> > + *
> > + * For the PRE1 pipe the de-gamma curves values are 24-bits, for the PRE2 pipe
> > + * the de-gamma curve values are 12-bits.
> > + *
> > + * For the PRE1 pipe de-gamma module sampling points @dx values are in the range
> > + * [0, 15] (4 bits). For the PRE2 pipe de-gamma module sampling points values
> > + * are in the range [0, 7] (3 bits).
> > + *
> > + * Userspace is expected to provide the curve values and sampling points with a
> > + * bit-depth matching the one of pipe in use.
> > + *
> > + * @header: block header (type = RPPX1_PARAMS_BLOCK_TYPE_LIN_PRE1 or
> > + *	    RPPX1_PARAMS_BLOCK_TYPE_LIN_PRE2)
> > + * @curve_r: de-gamma linearization curve for red channel
> > + * @curve_g: de-gamma linearization curve for green channel
> > + * @curve_b: de-gamma linearization curve for blue channel
> > + * @dx: input sampling points
> > + */
> > +struct rppx1_lin_params {
> > +	struct v4l2_isp_params_block_header header;
> > +	__u32 curve_r[RPPX1_LIN_DEGAMMA_CURVE_NUM];
> > +	__u32 curve_g[RPPX1_LIN_DEGAMMA_CURVE_NUM];
> > +	__u32 curve_b[RPPX1_LIN_DEGAMMA_CURVE_NUM];
> > +	__u8 dx[RPPX1_LIN_SAMPLE_POINTS_NUM];
> > +};
> > +
> >  /**
> >   * RPPX1_PARAMS_MAX_SIZE - Maximum size of all RPP-X1 parameter blocks
> >   *
> > @@ -568,7 +612,9 @@ struct rppx1_ga_params {
> >  	sizeof(struct rppx1_lsc_params)				+	\
> >  	sizeof(struct rppx1_lsc_params)				+	\
> >  	sizeof(struct rppx1_ga_params)				+	\
> > -	sizeof(struct rppx1_ga_params))
> > +	sizeof(struct rppx1_ga_params)				+	\
> > +	sizeof(struct rppx1_lin_params)				+	\
> > +	sizeof(struct rppx1_lin_params))
> 
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> Thanks
>   j
> 
> >
> >  /* ---------------------------------------------------------------------------
> >   * Statistics Structures
> > --
> > 2.54.0
> >

-- 
Kind Regards,
Niklas Söderlund