From: "Niklas Söderlund" <niklas.soderlund@ragnatech.se>
To: "Barnabás Pőcze" <barnabas.pocze+renesas@ideasonboard.com>
Cc: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>,
Jai Luthra <jai.luthra+renesas@ideasonboard.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] media: rppx1: lsc: Fix and use LSC_SIZE_VALUE()
Date: Fri, 2 Oct 2026 17:18:58 +0200 [thread overview]
Message-ID: <20261002151858.GA1933679@ragnatech.se> (raw)
In-Reply-To: <20261002121643.418200-1-barnabas.pocze+renesas@ideasonboard.com>
Hi Barnabás,
Nice catch!
On 2026-10-02 14:16:43 +0200, Barnabás Pőcze wrote:
> Firstly, the size values are 10-bit unsigned integers, and there doesn't
> appear to be an upper limit in the hardware documentation, and testing also
> seems to confirm that 1023 works as expected, so the correct mask to use is
> 0x3ff (1023), not 0x1ff (511).
I have the fields (x_sect_size_{0,1}) in RPP_MAIN_PRE1_LSC_XSIZE_01
defined as 10-bits so it is documented right? The thing here is that the
incorrect define LSC_GRAD_VALUE was used where LSC_SIZE_VALUE should
have, and this masked the error, no?
>
> Secondly, actually use `LSC_SIZE_VALUE()` when populating the size registers
> instead of using the `LSC_GRAD_VALUE()` macro.
>
> Fixes: b39656efb71a ("media: rppx1: lsc: Add support for lens shade correction")
> Signed-off-by: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
This fixes it correctly.
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> .../platform/dreamchip/rppx1/rppx1_lsc.c | 34 +++++++++----------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1_lsc.c b/drivers/media/platform/dreamchip/rppx1/rppx1_lsc.c
> index 8badeca23e249..ffc52ca23dd99 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rppx1_lsc.c
> +++ b/drivers/media/platform/dreamchip/rppx1/rppx1_lsc.c
> @@ -57,7 +57,7 @@
>
> #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))
> +#define LSC_SIZE_VALUE(v1, v2) (((v1) & 0x3ff) | (((v2) & 0x3ff) << 16))
>
> static int rppx1_lsc_probe(struct rpp_module *mod)
> {
> @@ -157,24 +157,24 @@ rppx1_lsc_fill_params(struct rpp_module *mod,
> 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]));
> + write(priv, mod->base + LSC_XSIZE_01_REG, LSC_SIZE_VALUE(v[0], v[1]));
> + write(priv, mod->base + LSC_XSIZE_23_REG, LSC_SIZE_VALUE(v[2], v[3]));
> + write(priv, mod->base + LSC_XSIZE_45_REG, LSC_SIZE_VALUE(v[4], v[5]));
> + write(priv, mod->base + LSC_XSIZE_67_REG, LSC_SIZE_VALUE(v[6], v[7]));
> + write(priv, mod->base + LSC_XSIZE_89_REG, LSC_SIZE_VALUE(v[8], v[9]));
> + write(priv, mod->base + LSC_XSIZE_1011_REG, LSC_SIZE_VALUE(v[10], v[11]));
> + write(priv, mod->base + LSC_XSIZE_1213_REG, LSC_SIZE_VALUE(v[12], v[13]));
> + write(priv, mod->base + LSC_XSIZE_1415_REG, LSC_SIZE_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]));
> + write(priv, mod->base + LSC_YSIZE_01_REG, LSC_SIZE_VALUE(v[0], v[1]));
> + write(priv, mod->base + LSC_YSIZE_23_REG, LSC_SIZE_VALUE(v[2], v[3]));
> + write(priv, mod->base + LSC_YSIZE_45_REG, LSC_SIZE_VALUE(v[4], v[5]));
> + write(priv, mod->base + LSC_YSIZE_67_REG, LSC_SIZE_VALUE(v[6], v[7]));
> + write(priv, mod->base + LSC_YSIZE_89_REG, LSC_SIZE_VALUE(v[8], v[9]));
> + write(priv, mod->base + LSC_YSIZE_1011_REG, LSC_SIZE_VALUE(v[10], v[11]));
> + write(priv, mod->base + LSC_YSIZE_1213_REG, LSC_SIZE_VALUE(v[12], v[13]));
> + write(priv, mod->base + LSC_YSIZE_1415_REG, LSC_SIZE_VALUE(v[14], v[15]));
>
> /* Enable module. */
> write(priv, mod->base + LSC_CTRL_REG, LSC_CTRL_LSC_EN);
> --
> 2.56.0
>
--
Kind Regards,
Niklas Söderlund
next prev parent reply other threads:[~2026-10-02 15:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-02 12:16 [PATCH v1] media: rppx1: lsc: Fix and use LSC_SIZE_VALUE() Barnabás Pőcze
2026-10-02 14:59 ` Jai Luthra
2026-10-02 15:18 ` Niklas Söderlund [this message]
2026-10-02 15:27 ` Barnabás Pőcze
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=20261002151858.GA1933679@ragnatech.se \
--to=niklas.soderlund@ragnatech.se \
--cc=barnabas.pocze+renesas@ideasonboard.com \
--cc=jacopo.mondi+renesas@ideasonboard.com \
--cc=jai.luthra+renesas@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=sakari.ailus@linux.intel.com \
/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.