[PATCH 0/2] media: rppx1: parameters block fixes

[PATCH 0/2] media: rppx1: parameters block fixes

[Linmao Li]

Fix two issues in the RPP-X1 parameters path.

Patch 1 adds the missing type entry and dispatch for the MAIN_POST
white balance gains block. Without them, userspace cannot configure
the module. The zero-sized block this hole exposes in the common
validator is handled by a pending v4l2-isp patch.

Patch 2 fixes the Bayer pattern source used by PRE2 black level
compensation. The PRE2 path is not enabled yet, so this is a latent
fix.

The series is based on media-committers next at 31152f5b0f87.

Compile tested with allmodconfig. W=1 builds of drivers/media/v4l2-core,
drivers/media/platform/dreamchip and
drivers/media/platform/renesas/rcar-isp were clean before and after the
series. I have no RPP-X1 hardware, so neither patch is runtime tested.

Linmao Li (2):
  media: rppx1: handle the MAIN_POST white balance gains block
  media: rppx1: bls: read the raw pattern from the PRE2 acquisition
    module

 drivers/media/platform/dreamchip/rppx1/rpp_params.c | 4 ++++
 drivers/media/platform/dreamchip/rppx1/rppx1_bls.c  | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH 1/2] media: rppx1: handle the MAIN_POST white balance gains block

[Linmao Li]

RPPX1_PARAMS_BLOCK_TYPE_AWBG_POST is defined in the uAPI, the MAIN_POST
white balance gains module is probed and started, and
RPPX1_PARAMS_MAX_SIZE already reserves room for all three white balance
gains blocks. However, the parameters type table has no entry for
AWBG_POST and rppx1_params() does not dispatch it, so userspace cannot
configure the module.

The missing type-table entry is zero-initialised. A zero-sized
AWBG_POST block can therefore make v4l2_isp_params_validate_buffer()
loop forever. A pending v4l2-isp patch rejects zero-sized blocks in the
common validator.

Add the missing type entry and dispatch AWBG_POST to rpp->post.awbg.

Fixes: 9ebf50010c68 ("media: rppx1: awbg: Add support for white balance gain settings")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/media/platform/dreamchip/rppx1/rpp_params.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
index a75a27a8afd09..88162f0bdc115 100644
--- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
+++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
@@ -25,6 +25,7 @@ rppx1_ext_params_blocks_info[] = {
 	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(AWBG_POST, awbg),
 	RPPX1_PARAMS_BLOCK_INFO(CCOR_POST, ccor),
 	RPPX1_PARAMS_BLOCK_INFO(HIST_PRE1, hist),
 	RPPX1_PARAMS_BLOCK_INFO(HIST_PRE2, hist),
@@ -79,6 +80,9 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb, size_t max_size,
 		case RPPX1_PARAMS_BLOCK_TYPE_AWBG_PRE1:
 			module = &rpp->pre1.awbg;
 			break;
+		case RPPX1_PARAMS_BLOCK_TYPE_AWBG_POST:
+			module = &rpp->post.awbg;
+			break;
 		case RPPX1_PARAMS_BLOCK_TYPE_CCOR_POST:
 			module = &rpp->post.ccor;
 			break;
-- 
2.25.1

Re: [PATCH 1/2] media: rppx1: handle the MAIN_POST white balance gains block

[Niklas Söderlund]

Hi Linmao,

Thanks for your work.

On 2026-08-17 18:45:28 +0800, Linmao Li wrote:
> RPPX1_PARAMS_BLOCK_TYPE_AWBG_POST is defined in the uAPI, the MAIN_POST
> white balance gains module is probed and started, and
> RPPX1_PARAMS_MAX_SIZE already reserves room for all three white balance
> gains blocks. However, the parameters type table has no entry for
> AWBG_POST and rppx1_params() does not dispatch it, so userspace cannot
> configure the module.
> 
> The missing type-table entry is zero-initialised. A zero-sized
> AWBG_POST block can therefore make v4l2_isp_params_validate_buffer()
> loop forever. A pending v4l2-isp patch rejects zero-sized blocks in the
> common validator.
> 
> Add the missing type entry and dispatch AWBG_POST to rpp->post.awbg.
> 
> Fixes: 9ebf50010c68 ("media: rppx1: awbg: Add support for white balance gain settings")
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> ---
>  drivers/media/platform/dreamchip/rppx1/rpp_params.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> index a75a27a8afd09..88162f0bdc115 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> +++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> @@ -25,6 +25,7 @@ rppx1_ext_params_blocks_info[] = {
>  	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(AWBG_POST, awbg),

This looks good.

>  	RPPX1_PARAMS_BLOCK_INFO(CCOR_POST, ccor),
>  	RPPX1_PARAMS_BLOCK_INFO(HIST_PRE1, hist),
>  	RPPX1_PARAMS_BLOCK_INFO(HIST_PRE2, hist),
> @@ -79,6 +80,9 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb, size_t max_size,
>  		case RPPX1_PARAMS_BLOCK_TYPE_AWBG_PRE1:
>  			module = &rpp->pre1.awbg;
>  			break;
> +		case RPPX1_PARAMS_BLOCK_TYPE_AWBG_POST:
> +			module = &rpp->post.awbg;
> +			break;

As you point out in the cover letter there are many modules, specially 
in the PRE2 pipeline, that we reserve space for in the configuration 
buffer but to not dispatch yet. But also some in the POST pipeline. The 
reason for this is that we have no users (libcamera) or test-cases for 
those blocks.

I think I would prefers keeping it like this until we do. If you really 
really want to go this path should you not also add AWBG_PRE2 here, that 
is also not dispatched ;-) But I think you should drop this and just add 
the missing entry to rppx1_ext_params_blocks_info.

>  		case RPPX1_PARAMS_BLOCK_TYPE_CCOR_POST:
>  			module = &rpp->post.ccor;
>  			break;
> -- 
> 2.25.1
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH 1/2] media: rppx1: handle the MAIN_POST white balance gains block

[Jacopo Mondi]

Hello Linmao, Niklas

On Mon, Aug 17, 2026 at 05:49:52PM +0200, Niklas Söderlund wrote:
> Hi Linmao,
>
> Thanks for your work.
>
> On 2026-08-17 18:45:28 +0800, Linmao Li wrote:
> > RPPX1_PARAMS_BLOCK_TYPE_AWBG_POST is defined in the uAPI, the MAIN_POST
> > white balance gains module is probed and started, and
> > RPPX1_PARAMS_MAX_SIZE already reserves room for all three white balance
> > gains blocks. However, the parameters type table has no entry for
> > AWBG_POST and rppx1_params() does not dispatch it, so userspace cannot
> > configure the module.
> >
> > The missing type-table entry is zero-initialised. A zero-sized
> > AWBG_POST block can therefore make v4l2_isp_params_validate_buffer()
> > loop forever. A pending v4l2-isp patch rejects zero-sized blocks in the
> > common validator.
> >
> > Add the missing type entry and dispatch AWBG_POST to rpp->post.awbg.
> >
> > Fixes: 9ebf50010c68 ("media: rppx1: awbg: Add support for white balance gain settings")
> > Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> > ---
> >  drivers/media/platform/dreamchip/rppx1/rpp_params.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> > index a75a27a8afd09..88162f0bdc115 100644
> > --- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> > +++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> > @@ -25,6 +25,7 @@ rppx1_ext_params_blocks_info[] = {
> >  	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(AWBG_POST, awbg),
>
> This looks good.
>
> >  	RPPX1_PARAMS_BLOCK_INFO(CCOR_POST, ccor),
> >  	RPPX1_PARAMS_BLOCK_INFO(HIST_PRE1, hist),
> >  	RPPX1_PARAMS_BLOCK_INFO(HIST_PRE2, hist),
> > @@ -79,6 +80,9 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb, size_t max_size,
> >  		case RPPX1_PARAMS_BLOCK_TYPE_AWBG_PRE1:
> >  			module = &rpp->pre1.awbg;
> >  			break;
> > +		case RPPX1_PARAMS_BLOCK_TYPE_AWBG_POST:
> > +			module = &rpp->post.awbg;
> > +			break;
>
> As you point out in the cover letter there are many modules, specially
> in the PRE2 pipeline, that we reserve space for in the configuration
> buffer but to not dispatch yet. But also some in the POST pipeline. The
> reason for this is that we have no users (libcamera) or test-cases for
> those blocks.
>
> I think I would prefers keeping it like this until we do. If you really
> really want to go this path should you not also add AWBG_PRE2 here, that
> is also not dispatched ;-) But I think you should drop this and just add
> the missing entry to rppx1_ext_params_blocks_info.

FYI: https://patchwork.linuxtv.org/project/linux-media/patch/20260815193839.141406-1-devnexen@gmail.com/

I feel like we should populate all blocks indeed to avoid holes in the
rppx1_ext_params_blocks_info[] array.

If we can work out a patch quick enough we can send it as a fix and
have it collected with the rpp-x1 driver for v7.3 (hence, no need for
a fixes tag).

Thanks
  j

>
> >  		case RPPX1_PARAMS_BLOCK_TYPE_CCOR_POST:
> >  			module = &rpp->post.ccor;
> >  			break;
> > --
> > 2.25.1
> >
>
> --
> Kind Regards,
> Niklas Söderlund
>

[PATCH 2/2] media: rppx1: bls: read the raw pattern from the PRE2 acquisition module

[Linmao Li]

rppx1_bls_swap_regs() gets the Bayer pattern from the acquisition
module. The PRE1 path uses pre1.acq, but the PRE2 path mistakenly uses
pre2.bls. The BLS module does not store a Bayer pattern, so PRE2 would
read zero (RGGB) and map fixed black levels to the wrong colour
registers.

PRE2 is not started or dispatched yet, so the bug is currently
latent. Read the pattern from pre2.acq, as the PRE1 path does.

Fixes: 3f8a2a883655 ("media: rppx1: bls: Add support for black level compensation")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/media/platform/dreamchip/rppx1/rppx1_bls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
index 01a61db279bf7..71c5561457d52 100644
--- a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
+++ b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
@@ -70,7 +70,7 @@ rppx1_bls_swap_regs(struct rpp_module *mod, const u32 input[4], u32 output[4])
 
 	/* 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;
+		&mod->rpp->pre1.acq : &mod->rpp->pre2.acq;
 	enum rpp_raw_pattern pattern = acq->info.acq.raw_pattern;
 
 	for (unsigned int i = 0; i < 4; ++i)
-- 
2.25.1

Re: [PATCH 2/2] media: rppx1: bls: read the raw pattern from the PRE2 acquisition module

[Niklas Söderlund]

Hello Linmao,

Thanks for your work.

On 2026-08-17 18:45:29 +0800, Linmao Li wrote:
> rppx1_bls_swap_regs() gets the Bayer pattern from the acquisition
> module. The PRE1 path uses pre1.acq, but the PRE2 path mistakenly uses
> pre2.bls. The BLS module does not store a Bayer pattern, so PRE2 would
> read zero (RGGB) and map fixed black levels to the wrong colour
> registers.
> 
> PRE2 is not started or dispatched yet, so the bug is currently
> latent. Read the pattern from pre2.acq, as the PRE1 path does.
> 
> Fixes: 3f8a2a883655 ("media: rppx1: bls: Add support for black level compensation")
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> ---
>  drivers/media/platform/dreamchip/rppx1/rppx1_bls.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> index 01a61db279bf7..71c5561457d52 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> +++ b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> @@ -70,7 +70,7 @@ rppx1_bls_swap_regs(struct rpp_module *mod, const u32 input[4], u32 output[4])
>  
>  	/* 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;
> +		&mod->rpp->pre1.acq : &mod->rpp->pre2.acq;

As pointed out elsewhere in this series, PRE2 is not enabled yet.

>  	enum rpp_raw_pattern pattern = acq->info.acq.raw_pattern;
>  
>  	for (unsigned int i = 0; i < 4; ++i)
> -- 
> 2.25.1
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH 2/2] media: rppx1: bls: read the raw pattern from the PRE2 acquisition module

[Jacopo Mondi]

Hi Niklas

On Mon, Aug 17, 2026 at 07:14:12PM +0200, Niklas Söderlund wrote:
> Hello Linmao,
>
> Thanks for your work.
>
> On 2026-08-17 18:45:29 +0800, Linmao Li wrote:
> > rppx1_bls_swap_regs() gets the Bayer pattern from the acquisition
> > module. The PRE1 path uses pre1.acq, but the PRE2 path mistakenly uses
> > pre2.bls. The BLS module does not store a Bayer pattern, so PRE2 would
> > read zero (RGGB) and map fixed black levels to the wrong colour
> > registers.
> >
> > PRE2 is not started or dispatched yet, so the bug is currently
> > latent. Read the pattern from pre2.acq, as the PRE1 path does.
> >
> > Fixes: 3f8a2a883655 ("media: rppx1: bls: Add support for black level compensation")
> > Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> > ---
> >  drivers/media/platform/dreamchip/rppx1/rppx1_bls.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > index 01a61db279bf7..71c5561457d52 100644
> > --- a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > +++ b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > @@ -70,7 +70,7 @@ rppx1_bls_swap_regs(struct rpp_module *mod, const u32 input[4], u32 output[4])
> >
> >  	/* 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;
> > +		&mod->rpp->pre1.acq : &mod->rpp->pre2.acq;
>
> As pointed out elsewhere in this series, PRE2 is not enabled yet.
>

Isn't this however a bug worth fixing anyway ?

The raw_pattern is initialized for the 'acq' module, not the bls one

rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_BGGR;
rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_GBRG;
rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_GRBG;
rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_RGGB;


> >  	enum rpp_raw_pattern pattern = acq->info.acq.raw_pattern;
> >
> >  	for (unsigned int i = 0; i < 4; ++i)
> > --
> > 2.25.1
> >
>
> --
> Kind Regards,
> Niklas Söderlund

Re: [PATCH 2/2] media: rppx1: bls: read the raw pattern from the PRE2 acquisition module

[Niklas Söderlund]

On 2026-08-18 08:25:29 +0200, Jacopo Mondi wrote:
> Hi Niklas
> 
> On Mon, Aug 17, 2026 at 07:14:12PM +0200, Niklas Söderlund wrote:
> > Hello Linmao,
> >
> > Thanks for your work.
> >
> > On 2026-08-17 18:45:29 +0800, Linmao Li wrote:
> > > rppx1_bls_swap_regs() gets the Bayer pattern from the acquisition
> > > module. The PRE1 path uses pre1.acq, but the PRE2 path mistakenly uses
> > > pre2.bls. The BLS module does not store a Bayer pattern, so PRE2 would
> > > read zero (RGGB) and map fixed black levels to the wrong colour
> > > registers.
> > >
> > > PRE2 is not started or dispatched yet, so the bug is currently
> > > latent. Read the pattern from pre2.acq, as the PRE1 path does.
> > >
> > > Fixes: 3f8a2a883655 ("media: rppx1: bls: Add support for black level compensation")
> > > Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> > > ---
> > >  drivers/media/platform/dreamchip/rppx1/rppx1_bls.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > > index 01a61db279bf7..71c5561457d52 100644
> > > --- a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > > +++ b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > > @@ -70,7 +70,7 @@ rppx1_bls_swap_regs(struct rpp_module *mod, const u32 input[4], u32 output[4])
> > >
> > >  	/* 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;
> > > +		&mod->rpp->pre1.acq : &mod->rpp->pre2.acq;
> >
> > As pointed out elsewhere in this series, PRE2 is not enabled yet.
> >
> 
> Isn't this however a bug worth fixing anyway ?

We could. I was under the impression we did not want to enable stuff 
that did not have a user. If you are OK with this so am I.

> 
> The raw_pattern is initialized for the 'acq' module, not the bls one
> 
> rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_BGGR;
> rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_GBRG;
> rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_GRBG;
> rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_RGGB;
> 
> 
> > >  	enum rpp_raw_pattern pattern = acq->info.acq.raw_pattern;
> > >
> > >  	for (unsigned int i = 0; i < 4; ++i)
> > > --
> > > 2.25.1
> > >
> >
> > --
> > Kind Regards,
> > Niklas Söderlund

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH 2/2] media: rppx1: bls: read the raw pattern from the PRE2 acquisition module

[Jacopo Mondi]

Hi Niklas,

On Tue, Aug 18, 2026 at 10:20:14AM +0200, Niklas Söderlund wrote:
> On 2026-08-18 08:25:29 +0200, Jacopo Mondi wrote:
> > Hi Niklas
> >
> > On Mon, Aug 17, 2026 at 07:14:12PM +0200, Niklas Söderlund wrote:
> > > Hello Linmao,
> > >
> > > Thanks for your work.
> > >
> > > On 2026-08-17 18:45:29 +0800, Linmao Li wrote:
> > > > rppx1_bls_swap_regs() gets the Bayer pattern from the acquisition
> > > > module. The PRE1 path uses pre1.acq, but the PRE2 path mistakenly uses
> > > > pre2.bls. The BLS module does not store a Bayer pattern, so PRE2 would
> > > > read zero (RGGB) and map fixed black levels to the wrong colour
> > > > registers.
> > > >
> > > > PRE2 is not started or dispatched yet, so the bug is currently
> > > > latent. Read the pattern from pre2.acq, as the PRE1 path does.
> > > >
> > > > Fixes: 3f8a2a883655 ("media: rppx1: bls: Add support for black level compensation")
> > > > Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> > > > ---
> > > >  drivers/media/platform/dreamchip/rppx1/rppx1_bls.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > > > index 01a61db279bf7..71c5561457d52 100644
> > > > --- a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > > > +++ b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > > > @@ -70,7 +70,7 @@ rppx1_bls_swap_regs(struct rpp_module *mod, const u32 input[4], u32 output[4])
> > > >
> > > >  	/* 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;
> > > > +		&mod->rpp->pre1.acq : &mod->rpp->pre2.acq;
> > >
> > > As pointed out elsewhere in this series, PRE2 is not enabled yet.
> > >
> >
> > Isn't this however a bug worth fixing anyway ?
>
> We could. I was under the impression we did not want to enable stuff
> that did not have a user. If you are OK with this so am I.

Unless I'm missing something obvious this is not about enabling the
module, but rather making sure that the raw_pattern information will
be accessed from the right place (pre2.acq) rather than from pre2.bls
where they are not initialized.

True, at the moment this path is not exercized, but I would rather fix
it right away rather than waiting for the pre2 pipe to be enabled and
finding out it is broken :)

As a bonus point, if we fix it right away, it might end up in 7.3
along with the driver itself.

>
> >
> > The raw_pattern is initialized for the 'acq' module, not the bls one
> >
> > rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_BGGR;
> > rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_GBRG;
> > rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_GRBG;
> > rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_RGGB;
> >
> >
> > > >  	enum rpp_raw_pattern pattern = acq->info.acq.raw_pattern;
> > > >
> > > >  	for (unsigned int i = 0; i < 4; ++i)
> > > > --
> > > > 2.25.1
> > > >
> > >
> > > --
> > > Kind Regards,
> > > Niklas Söderlund
>
> --
> Kind Regards,
> Niklas Söderlund
>

Re: [PATCH 2/2] media: rppx1: bls: read the raw pattern from the PRE2 acquisition module

[Niklas Söderlund]

Hi Jacopo,

On 2026-08-18 10:41:06 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Tue, Aug 18, 2026 at 10:20:14AM +0200, Niklas Söderlund wrote:
> > On 2026-08-18 08:25:29 +0200, Jacopo Mondi wrote:
> > > Hi Niklas
> > >
> > > On Mon, Aug 17, 2026 at 07:14:12PM +0200, Niklas Söderlund wrote:
> > > > Hello Linmao,
> > > >
> > > > Thanks for your work.
> > > >
> > > > On 2026-08-17 18:45:29 +0800, Linmao Li wrote:
> > > > > rppx1_bls_swap_regs() gets the Bayer pattern from the acquisition
> > > > > module. The PRE1 path uses pre1.acq, but the PRE2 path mistakenly uses
> > > > > pre2.bls. The BLS module does not store a Bayer pattern, so PRE2 would
> > > > > read zero (RGGB) and map fixed black levels to the wrong colour
> > > > > registers.
> > > > >
> > > > > PRE2 is not started or dispatched yet, so the bug is currently
> > > > > latent. Read the pattern from pre2.acq, as the PRE1 path does.
> > > > >
> > > > > Fixes: 3f8a2a883655 ("media: rppx1: bls: Add support for black level compensation")
> > > > > Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> > > > > ---
> > > > >  drivers/media/platform/dreamchip/rppx1/rppx1_bls.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > > > > index 01a61db279bf7..71c5561457d52 100644
> > > > > --- a/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > > > > +++ b/drivers/media/platform/dreamchip/rppx1/rppx1_bls.c
> > > > > @@ -70,7 +70,7 @@ rppx1_bls_swap_regs(struct rpp_module *mod, const u32 input[4], u32 output[4])
> > > > >
> > > > >  	/* 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;
> > > > > +		&mod->rpp->pre1.acq : &mod->rpp->pre2.acq;
> > > >
> > > > As pointed out elsewhere in this series, PRE2 is not enabled yet.
> > > >
> > >
> > > Isn't this however a bug worth fixing anyway ?
> >
> > We could. I was under the impression we did not want to enable stuff
> > that did not have a user. If you are OK with this so am I.
> 
> Unless I'm missing something obvious this is not about enabling the
> module, but rather making sure that the raw_pattern information will
> be accessed from the right place (pre2.acq) rather than from pre2.bls
> where they are not initialized.
> 
> True, at the moment this path is not exercized, but I would rather fix
> it right away rather than waiting for the pre2 pipe to be enabled and
> finding out it is broken :)
> 
> As a bonus point, if we fix it right away, it might end up in 7.3
> along with the driver itself.

Sounds good.

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> 
> >
> > >
> > > The raw_pattern is initialized for the 'acq' module, not the bls one
> > >
> > > rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_BGGR;
> > > rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_GBRG;
> > > rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_GRBG;
> > > rppx1/rppx1_acq.c:             mod->info.acq.raw_pattern = RPP_RGGB;
> > >
> > >
> > > > >  	enum rpp_raw_pattern pattern = acq->info.acq.raw_pattern;
> > > > >
> > > > >  	for (unsigned int i = 0; i < 4; ++i)
> > > > > --
> > > > > 2.25.1
> > > > >
> > > >
> > > > --
> > > > Kind Regards,
> > > > Niklas Söderlund
> >
> > --
> > Kind Regards,
> > Niklas Söderlund
> >

-- 
Kind Regards,
Niklas Söderlund