Re: [PATCH 05/11] media: i2c: ov5640: Remove unsupported bayer orders

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

Hi Kieran

On Thu, May 14, 2026 at 09:43:42AM +0100, Kieran Bingham wrote:
> Quoting Jacopo Mondi (2026-05-14 09:10:48)
> > Hi Kieran
> >
> > On Fri, May 01, 2026 at 04:39:07PM +0100, Kieran Bingham wrote:
> > > The OV5640 only outputs SBGGR8. Remove the incorrectly advertised
> > > alternatives which allow a misconfigured pipeline to be established.
> >
> > Do you have any idea why the datasheet mentions all the RGGB
> > permutations as valid outputs ?
>
> I would anticipate it's because the internal ISP component can support them.
>

Note that an analogue crop rectangle not aligned with the Bayer macro-pixel
could change the pattern, but the sensor's ISP shouldn't need to be
informed afaiu

> But now I think more I have to admit, I have no idea if other modules
> might have different physical orderings.
>
> But certainly as far as I can tell any given module can only have a
> single 'correct' value here.

analogue crop alignment apart :)

>
> So we'll have to try to work out how to confirm/verify that perhaps ?
>
> >
> > >
> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > ---
> > >  drivers/media/i2c/ov5640.c | 21 ---------------------
> > >  1 file changed, 21 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > > index 244c341d0e77..e1e253730206 100644
> > > --- a/drivers/media/i2c/ov5640.c
> > > +++ b/drivers/media/i2c/ov5640.c
> > > @@ -309,27 +309,6 @@ static const struct ov5640_pixfmt ov5640_csi2_formats[] = {
> > >               .bpp            = 8,
> > >               .ctrl00         = 0x00,
> > >               .mux            = OV5640_FMT_MUX_RAW_DPC,
> > > -     }, {
> > > -             /* Raw bayer, GBGB... / RGRG... */
> > > -             .code           = MEDIA_BUS_FMT_SGBRG8_1X8,
> > > -             .colorspace     = V4L2_COLORSPACE_SRGB,
> > > -             .bpp            = 8,
> > > -             .ctrl00         = 0x01,
> > > -             .mux            = OV5640_FMT_MUX_RAW_DPC,
> > > -     }, {
> > > -             /* Raw bayer, GRGR... / BGBG... */
> > > -             .code           = MEDIA_BUS_FMT_SGRBG8_1X8,
> > > -             .colorspace     = V4L2_COLORSPACE_SRGB,
> > > -             .bpp            = 8,
> > > -             .ctrl00         = 0x02,
> > > -             .mux            = OV5640_FMT_MUX_RAW_DPC,
> > > -     }, {
> > > -             /* Raw bayer, RGRG... / GBGB... */
> > > -             .code           = MEDIA_BUS_FMT_SRGGB8_1X8,
> > > -             .colorspace     = V4L2_COLORSPACE_SRGB,
> > > -             .bpp            = 8,
> > > -             .ctrl00         = 0x03,
> > > -             .mux            = OV5640_FMT_MUX_RAW_DPC,
> > >       },
> >
> > Seems like you've missed the same entries in the ov5640_dvp_formats[]
> > table.
>
> That was intentional so far as I can only test the CSI variant. But
> perhaps for the same reasons mentioned above, whatever happens on one
> will be the same on the other.
>
> I hope posting this will find some wider testers 'perhaps' if anyone
> cares.
>
> --
> Kieran.
>
> >
> > >       { /* sentinel */ }
> > >  };
> > >
> > > --
> > > 2.52.0
> > >
> > >