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

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

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
>