All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
To: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>
Cc: Jacopo Mondi <jacopo.mondi@ideasonboard.com>,
	 Jai Luthra <jai.luthra+renesas@ideasonboard.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	 Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	 linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	 Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
Subject: Re: [PATCH v9 11/13] media: rppx1: lsc: Add support for lens shade correction
Date: Wed, 3 Jun 2026 16:32:12 +0200	[thread overview]
Message-ID: <aiA6lR6TeNHAxSFH@zed> (raw)
In-Reply-To: <20260516211320.3041412-12-niklas.soderlund+renesas@ragnatech.se>

Hi Niklas

On Sat, May 16, 2026 at 11:13:18PM +0200, Niklas Söderlund wrote:
> Extend the RPPX1 driver to allow setting the lens shade correction
> 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: 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>
> Co-developed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---
>  .../platform/dreamchip/rppx1/rpp_module.h     |   1 +
>  .../platform/dreamchip/rppx1/rpp_params.c     |   5 +
>  .../platform/dreamchip/rppx1/rppx1_lsc.c      | 119 ++++++++++++++++++
>  .../uapi/linux/media/dreamchip/rppx1-config.h |  54 +++++++-
>  4 files changed, 178 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_module.h b/drivers/media/platform/dreamchip/rppx1/rpp_module.h
> index e039746ac542..48b61b5c35b4 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_lsc_params lsc;
>  	struct rppx1_awbg_params awbg;
>  	struct rppx1_ccor_params ccor;
>  	struct rppx1_hist_params hist;
> diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> index a83d393d0504..8d85d0c7bff1 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(LSC_PRE1, lsc),
> +	RPPX1_PARAMS_BLOCK_INFO(LSC_PRE2, lsc),
>  	RPPX1_PARAMS_BLOCK_INFO(AWBG_PRE1, awbg),
>  	RPPX1_PARAMS_BLOCK_INFO(AWBG_PRE2, awbg),
>  	RPPX1_PARAMS_BLOCK_INFO(CCOR_POST, ccor),
> @@ -64,6 +66,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_LSC_PRE1:
> +			module = &rpp->pre1.lsc;
> +			break;
>  		case RPPX1_PARAMS_BLOCK_TYPE_AWBG_PRE1:
>  			module = &rpp->pre1.awbg;
>  			break;
> diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1_lsc.c b/drivers/media/platform/dreamchip/rppx1/rppx1_lsc.c
> index be49fc17ea26..8badeca23e24 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rppx1_lsc.c
> +++ b/drivers/media/platform/dreamchip/rppx1/rppx1_lsc.c
> @@ -55,6 +55,10 @@
>  #define LSC_TABLE_SEL_REG	0x00a8
>  #define LSC_STATUS_REG		0x00ac
>
> +#define LSC_R_TABLE_DATA_VALUE(v1, v2) (((v1) & 0xfff) | (((v2) & 0xfff) << 12))
> +#define LSC_GRAD_VALUE(v1, v2) (((v1) & 0xfff) | (((v2) & 0xfff) << 16))
> +#define LSC_SIZE_VALUE(v1, v2) (((v1) & 0x1ff) | (((v2) & 0x1ff) << 16))
> +
>  static int rppx1_lsc_probe(struct rpp_module *mod)
>  {
>  	/* Version check. */
> @@ -64,6 +68,121 @@ static int rppx1_lsc_probe(struct rpp_module *mod)
>  	return 0;
>  }
>
> +static int
> +rppx1_lsc_fill_params(struct rpp_module *mod,
> +		      const union rppx1_params_block *block,
> +		      rppx1_reg_write write, void *priv)
> +{
> +	const struct rppx1_lsc_params *cfg = &block->lsc;
> +	const __u16 *v;
> +
> +	/* Always disable module as it needs be disabled before configuring. */
> +	write(priv, mod->base + LSC_CTRL_REG, 0);
> +	if (cfg->header.flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE)
> +		return 0;
> +
> +	/*
> +	 * Program the color correction sectors.
> +	 *
> +	 * There are two tables to one can program and switch between. As the
> +	 * RPPX1 supports preparing a buffer of commands to be applied later
> +	 * only use table 0. This works as long as the ISP is not used in
> +	 * inline-mode.
> +	 *
> +	 * For inline-mode support using DMA for configuration is not possible
> +	 * so this is not an issue, but needs to be address if inline-mode
> +	 * support is added to the driver.
> +	 */
> +
> +	/* Start writing at beginning of table 0. */
> +	write(priv, mod->base + LSC_R_TABLE_ADDR_REG, 0);
> +	write(priv, mod->base + LSC_GR_TABLE_ADDR_REG, 0);
> +	write(priv, mod->base + LSC_B_TABLE_ADDR_REG, 0);
> +	write(priv, mod->base + LSC_GB_TABLE_ADDR_REG, 0);
> +
> +	/* Program data tables. */
> +	for (unsigned int i = 0; i < RPPX1_LSC_SAMPLES_MAX; i++) {
> +		const __u16 *r = cfg->r_data[i];
> +		const __u16 *gr = cfg->gr_data[i];
> +		const __u16 *b = cfg->b_data[i];
> +		const __u16 *gb = cfg->gb_data[i];
> +		unsigned int j;
> +
> +		for (j = 0; j < RPPX1_LSC_SAMPLES_MAX - 1; j += 2) {
> +			write(priv, mod->base + LSC_R_TABLE_DATA_REG,
> +			      LSC_R_TABLE_DATA_VALUE(r[j], r[j + 1]));
> +			write(priv, mod->base + LSC_GR_TABLE_DATA_REG,
> +			      LSC_R_TABLE_DATA_VALUE(gr[j], gr[j + 1]));
> +			write(priv, mod->base + LSC_B_TABLE_DATA_REG,
> +			      LSC_R_TABLE_DATA_VALUE(b[j], b[j + 1]));
> +			write(priv, mod->base + LSC_GB_TABLE_DATA_REG,
> +			      LSC_R_TABLE_DATA_VALUE(gb[j], gb[j + 1]));
> +		}
> +
> +		write(priv, mod->base + LSC_R_TABLE_DATA_REG,
> +		      LSC_R_TABLE_DATA_VALUE(r[j], 0));
> +		write(priv, mod->base + LSC_GR_TABLE_DATA_REG,
> +		      LSC_R_TABLE_DATA_VALUE(gr[j], 0));
> +		write(priv, mod->base + LSC_B_TABLE_DATA_REG,
> +		      LSC_R_TABLE_DATA_VALUE(b[j], 0));
> +		write(priv, mod->base + LSC_GB_TABLE_DATA_REG,
> +		      LSC_R_TABLE_DATA_VALUE(gb[j], 0));
> +	}
> +
> +	/* Activate table 0. */
> +	write(priv, mod->base + LSC_TABLE_SEL_REG, 0);
> +
> +	/*
> +	 * Program X- and Y- sizes, and gradients.
> +	 */
> +
> +	v = cfg->x_grad;
> +	write(priv, mod->base + LSC_XGRAD_01_REG, LSC_GRAD_VALUE(v[0], v[1]));
> +	write(priv, mod->base + LSC_XGRAD_23_REG, LSC_GRAD_VALUE(v[2], v[3]));
> +	write(priv, mod->base + LSC_XGRAD_45_REG, LSC_GRAD_VALUE(v[4], v[5]));
> +	write(priv, mod->base + LSC_XGRAD_67_REG, LSC_GRAD_VALUE(v[6], v[7]));
> +	write(priv, mod->base + LSC_XGRAD_89_REG, LSC_GRAD_VALUE(v[8], v[9]));
> +	write(priv, mod->base + LSC_XGRAD_1011_REG, LSC_GRAD_VALUE(v[10], v[11]));
> +	write(priv, mod->base + LSC_XGRAD_1213_REG, LSC_GRAD_VALUE(v[12], v[13]));
> +	write(priv, mod->base + LSC_XGRAD_1415_REG, LSC_GRAD_VALUE(v[14], v[15]));
> +
> +	v = cfg->y_grad;
> +	write(priv, mod->base + LSC_YGRAD_01_REG, LSC_GRAD_VALUE(v[0], v[1]));
> +	write(priv, mod->base + LSC_YGRAD_23_REG, LSC_GRAD_VALUE(v[2], v[3]));
> +	write(priv, mod->base + LSC_YGRAD_45_REG, LSC_GRAD_VALUE(v[4], v[5]));
> +	write(priv, mod->base + LSC_YGRAD_67_REG, LSC_GRAD_VALUE(v[6], v[7]));
> +	write(priv, mod->base + LSC_YGRAD_89_REG, LSC_GRAD_VALUE(v[8], v[9]));
> +	write(priv, mod->base + LSC_YGRAD_1011_REG, LSC_GRAD_VALUE(v[10], v[11]));
> +	write(priv, mod->base + LSC_YGRAD_1213_REG, LSC_GRAD_VALUE(v[12], v[13]));
> +	write(priv, mod->base + LSC_YGRAD_1415_REG, LSC_GRAD_VALUE(v[14], v[15]));
> +
> +	v = cfg->x_sect_size;
> +	write(priv, mod->base + LSC_XSIZE_01_REG, LSC_GRAD_VALUE(v[0], v[1]));
> +	write(priv, mod->base + LSC_XSIZE_23_REG, LSC_GRAD_VALUE(v[2], v[3]));
> +	write(priv, mod->base + LSC_XSIZE_45_REG, LSC_GRAD_VALUE(v[4], v[5]));
> +	write(priv, mod->base + LSC_XSIZE_67_REG, LSC_GRAD_VALUE(v[6], v[7]));
> +	write(priv, mod->base + LSC_XSIZE_89_REG, LSC_GRAD_VALUE(v[8], v[9]));
> +	write(priv, mod->base + LSC_XSIZE_1011_REG, LSC_GRAD_VALUE(v[10], v[11]));
> +	write(priv, mod->base + LSC_XSIZE_1213_REG, LSC_GRAD_VALUE(v[12], v[13]));
> +	write(priv, mod->base + LSC_XSIZE_1415_REG, LSC_GRAD_VALUE(v[14], v[15]));
> +
> +	v = cfg->y_sect_size;
> +	write(priv, mod->base + LSC_YSIZE_01_REG, LSC_GRAD_VALUE(v[0], v[1]));
> +	write(priv, mod->base + LSC_YSIZE_23_REG, LSC_GRAD_VALUE(v[2], v[3]));
> +	write(priv, mod->base + LSC_YSIZE_45_REG, LSC_GRAD_VALUE(v[4], v[5]));
> +	write(priv, mod->base + LSC_YSIZE_67_REG, LSC_GRAD_VALUE(v[6], v[7]));
> +	write(priv, mod->base + LSC_YSIZE_89_REG, LSC_GRAD_VALUE(v[8], v[9]));
> +	write(priv, mod->base + LSC_YSIZE_1011_REG, LSC_GRAD_VALUE(v[10], v[11]));
> +	write(priv, mod->base + LSC_YSIZE_1213_REG, LSC_GRAD_VALUE(v[12], v[13]));
> +	write(priv, mod->base + LSC_YSIZE_1415_REG, LSC_GRAD_VALUE(v[14], v[15]));
> +
> +	/* Enable module. */
> +	write(priv, mod->base + LSC_CTRL_REG, LSC_CTRL_LSC_EN);
> +
> +	return 0;
> +}
> +
>  const struct rpp_module_ops rppx1_lsc_ops = {
>  	.probe = rppx1_lsc_probe,
> +	.fill_params = rppx1_lsc_fill_params,
>  };
> diff --git a/include/uapi/linux/media/dreamchip/rppx1-config.h b/include/uapi/linux/media/dreamchip/rppx1-config.h
> index 97c333c72d56..7ebcc00ace04 100644
> --- a/include/uapi/linux/media/dreamchip/rppx1-config.h
> +++ b/include/uapi/linux/media/dreamchip/rppx1-config.h
> @@ -89,6 +89,8 @@ enum rppx1_meas_chan {
>   * @RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE1: PRE1 pipe Black Level Subtraction
>   * @RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE2: PRE2 pipe Black Level Subtraction
>   * @RPPX1_PARAMS_BLOCK_TYPE_CCOR_POST: POST pipe Color Correction
> + * @RPPX1_PARAMS_BLOCK_TYPE_LSC_PRE1: PRE1 pipe Lens Shading Correction
> + * @RPPX1_PARAMS_BLOCK_TYPE_LSC_PRE2: PRE2 Lens Shading Correction

missing 'pipe' after PRE2. Sorry, my mistake probably

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>   */
>  enum rppx1_params_block_type {
>  	RPPX1_PARAMS_BLOCK_TYPE_WBMEAS_POST,
> @@ -103,6 +105,8 @@ enum rppx1_params_block_type {
>  	RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE1,
>  	RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE2,
>  	RPPX1_PARAMS_BLOCK_TYPE_CCOR_POST,
> +	RPPX1_PARAMS_BLOCK_TYPE_LSC_PRE1,
> +	RPPX1_PARAMS_BLOCK_TYPE_LSC_PRE2,
>  };
>
>  /**
> @@ -447,6 +451,52 @@ struct rppx1_ccor_params {
>  	__u32 offset[3];
>  };
>
> +/* Lens Shade Correction */
> +#define RPPX1_LSC_SAMPLES_MAX 17
> +#define RPPX1_LSC_NUM_SECTORS 16
> +
> +/**
> + * struct rppx1_lsc_params - Lens Shading Correction configuration
> + *
> + * The RPP-X1 Lens shading correction 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_LSC_PRE1 or
> + * RPPX1_PARAMS_BLOCK_TYPE_LSC_PRE2.
> + *
> + * The module applies per-color channel correction factors @r_data, @gr_data,
> + * @gb_data and @b_data as a 16x16 grid mapped on the image. The size of each
> + * grid segment is expressed by the @x_sect_size and @y_sect_size arrays.  Each
> + * segment shall be at least 8 pixels in size and the sum of all horizontal
> + * segments @x_sect_size shall match the input frame size width.
> + *
> + * The correction factors values are expressed as unsigned Q2.10 integers
> + * ranging from 1 to 3.999.
> + *
> + * Pre-calculated interpolation factors shall be provided in the @x_grad
> + * and @y_grad fields, expressed as 12 bits integer values.
> + *
> + * @header: block header (type = RPPX1_PARAMS_BLOCK_TYPE_LSC)
> + * @r_data: correction factors for the red channel in Q2.10 format
> + * @gr_data: correction factors for the green (red) channel in Q2.10 format
> + * @gb_data: correction factors for the green (blue) channel in Q2.10 format
> + * @b_data: correction factors for the blue channel in Q2.10 format
> + * @x_grad: Interpolation gradients for each horizontal sector (12 bits)
> + * @y_grad: Interpolation gradients for each vertical sector (12 bits)
> + * @x_sect_size: Horizontal sectors sizes
> + * @y_sect_size: Vertical sectors sizes
> + */
> +struct rppx1_lsc_params {
> +	struct v4l2_isp_params_block_header header;
> +	__u16 r_data[RPPX1_LSC_SAMPLES_MAX][RPPX1_LSC_SAMPLES_MAX];
> +	__u16 gr_data[RPPX1_LSC_SAMPLES_MAX][RPPX1_LSC_SAMPLES_MAX];
> +	__u16 gb_data[RPPX1_LSC_SAMPLES_MAX][RPPX1_LSC_SAMPLES_MAX];
> +	__u16 b_data[RPPX1_LSC_SAMPLES_MAX][RPPX1_LSC_SAMPLES_MAX];
> +	__u16 x_grad[RPPX1_LSC_NUM_SECTORS];
> +	__u16 y_grad[RPPX1_LSC_NUM_SECTORS];
> +	__u16 x_sect_size[RPPX1_LSC_NUM_SECTORS];
> +	__u16 y_sect_size[RPPX1_LSC_NUM_SECTORS];
> +};
> +
>  /**
>   * RPPX1_PARAMS_MAX_SIZE - Maximum size of all RPP-X1 parameter blocks
>   *
> @@ -465,7 +515,9 @@ struct rppx1_ccor_params {
>  	sizeof(struct rppx1_hist_params)			+	\
>  	sizeof(struct rppx1_bls_params)				+	\
>  	sizeof(struct rppx1_bls_params)				+	\
> -	sizeof(struct rppx1_ccor_params))
> +	sizeof(struct rppx1_ccor_params)			+	\
> +	sizeof(struct rppx1_lsc_params)				+	\
> +	sizeof(struct rppx1_lsc_params))
>
>  /* ---------------------------------------------------------------------------
>   * Statistics Structures
> --
> 2.54.0
>

  reply	other threads:[~2026-06-03 14:32 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-16 21:13 [PATCH v9 00/13] media: Add support for R-Car ISP using Dreamchip RPPX1 ISP Niklas Söderlund
2026-05-16 21:13 ` [PATCH v9 01/13] media: Add RPP_X1_PARAMS and RPP_X1_STATS meta formats Niklas Söderlund
2026-06-03 10:49   ` Jacopo Mondi
2026-06-03 12:48     ` Niklas Söderlund
2026-05-16 21:13 ` [PATCH v9 02/13] media: uapi: Add extensible param and stats blocks for RPPX1 Niklas Söderlund
2026-06-03 10:50   ` Jacopo Mondi
2026-06-03 12:55     ` Niklas Söderlund
2026-05-16 21:13 ` [PATCH v9 03/13] media: rppx1: Add framework to support Dreamchip RPPX1 ISP Niklas Söderlund
2026-06-03 10:56   ` Jacopo Mondi
2026-06-03 12:24   ` Jacopo Mondi
2026-06-03 13:47     ` Niklas Söderlund
2026-06-03 14:26       ` Jacopo Mondi
2026-06-03 15:17         ` Niklas Söderlund
2026-06-03 15:33           ` Jacopo Mondi
2026-05-16 21:13 ` [PATCH v9 04/13] media: rcar-isp: Add support for ISPCORE Niklas Söderlund
2026-06-03 13:15   ` Jacopo Mondi
2026-05-16 21:13 ` [PATCH v9 05/13] media: rppx1: wbmeas: Add support for white balance measurement Niklas Söderlund
2026-06-03 13:35   ` Jacopo Mondi
2026-05-16 21:13 ` [PATCH v9 06/13] media: rppx1: awbg: Add support for white balance gain settings Niklas Söderlund
2026-06-03 13:38   ` Jacopo Mondi
2026-05-16 21:13 ` [PATCH v9 07/13] media: rppx1: exm: Add support for exposure measurement Niklas Söderlund
2026-06-03 13:57   ` Jacopo Mondi
2026-05-16 21:13 ` [PATCH v9 08/13] media: rppx1: hist: Add support histogram measurement Niklas Söderlund
2026-06-03 14:11   ` Jacopo Mondi
2026-05-16 21:13 ` [PATCH v9 09/13] media: rppx1: bls: Add support for black level compensation Niklas Söderlund
2026-05-16 21:13 ` [PATCH v9 10/13] media: rppx1: ccor: Add support for color correction matrix Niklas Söderlund
2026-05-16 21:13 ` [PATCH v9 11/13] media: rppx1: lsc: Add support for lens shade correction Niklas Söderlund
2026-06-03 14:32   ` Jacopo Mondi [this message]
2026-05-16 21:13 ` [PATCH v9 12/13] media: rppx1: ga: Add support for gamma out correction Niklas Söderlund
2026-06-03 14:33   ` Jacopo Mondi
2026-05-16 21:13 ` [PATCH v9 13/13] media: rppx1: lin: Add support for gamma sensor linearization Niklas Söderlund
2026-06-03 14:36   ` Jacopo Mondi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aiA6lR6TeNHAxSFH@zed \
    --to=jacopo.mondi@ideasonboard.com \
    --cc=jacopo.mondi+renesas@ideasonboard.com \
    --cc=jai.luthra+renesas@ideasonboard.com \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.