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

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

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
>