Re: [PATCH] media: rkisp1: Fix Bayer demosaicing bypass

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

Hi Kieran

On Fri, Sep 11, 2026 at 03:27:50PM +0100, Kieran Bingham wrote:
> Quoting Jacopo Mondi (2026-09-11 14:56:19)
> > The RKISP1_CIF_ISP_DEMOSAIC_BYPASS bit, when set, bypasses the
> > demosaicing block on the RkISP1 ISP.
> >
> > The current implementation however clears the bit when demosaicing
> > have to be bypassed and sets it when demosaicing has to be enabled,
> > effectively inverting the bypass bit handling logic.
>
> Ouch.
>
> > Fix this by setting the bypass bit when disabling the demosaicing block,
> > and by clearing it instead when demosaicing has to be performed.
> >
> > The issue never manifested itself as libcamera hasn't an algorithm
> > to control Bayer demosaicing bypass yet.
> >
> > Fixes: 6c53a7b68c5d ("media: rkisp1: Implement extensible params support")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> > media: rkisp1: Fix demosaicing bypass
> > ---
> >  drivers/media/platform/rockchip/rkisp1/rkisp1-params.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > index 042b759eba62..496381962f1b 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > @@ -1854,8 +1854,8 @@ rkisp1_ext_params_bdm(struct rkisp1_params *params,
> >         const struct rkisp1_ext_params_bdm_config *bdm = &block->bdm;
> >
> >         if (bdm->header.flags & RKISP1_EXT_PARAMS_FL_BLOCK_DISABLE) {
> > -               rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> > -                                       RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> > +               rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> > +                                     RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> >                 return;
> >         }
> >
> > @@ -1863,8 +1863,8 @@ rkisp1_ext_params_bdm(struct rkisp1_params *params,
> >
> >         if ((bdm->header.flags & RKISP1_EXT_PARAMS_FL_BLOCK_ENABLE) &&
> >             !(params->enabled_blocks & BIT(bdm->header.type)))
> > -               rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> > -                                     RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> > +               rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> > +                                       RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
>
>
> I think this also opens us up to add mono formats as explicitly
> supported by the ISP and potentially set the demosaic defaulting to off
> in that instance?

mmm, I think userspace is in a better position to decide when to
bypass debayer instead of relying on auto-configuration of the ISP ?

I guess we'll discuss this when support for luma-only formats will be
added to the driver

>
> Anyway, Looks sane to me in this order.
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

thanks

>
> >  }
> >
> >  static void
> >
> > ---
> > base-commit: 27953c044974baf7e24dee3e9342fe0103dea80c
> > change-id: 20260911-imx8mp-demosaicing-bypass-bb284d9cca60
> >
> > Best regards,
> > --
> > Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >