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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®