Re: [PATCH] media: i2c: imx258: correct mode to GBGB/RGRG

[Krzysztof Kozlowski <krzk@kernel.org>]

On Wed, 28 Oct 2020 at 11:03, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> On Wed, Oct 28, 2020 at 10:56:55AM +0100, Krzysztof Kozlowski wrote:
> > On Wed, 28 Oct 2020 at 10:45, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > On Wed, 28 Oct 2020 at 10:43, Yeh, Andy <andy.yeh@intel.com> wrote:
> > > >
> > > > But the sensor settings for the original submission is to output GRBG Bayer RAW.
> > > >
> > > > Regards, Andy
> > >
> > > No, not to my knowledge. There are no settings for color output
> > > because it is fixed to GBGB/RGRG. I was looking a lot into this driver
> > > (I have few other problems with it, already few other patches posted)
> > > and I could not find a setting for this in datasheet. If you know the
> > > setting for the other color - can you point me to it?
> >
> > And except the datasheet which mentions the specific format, the
> > testing confirms it. With original color the pictures are pink/purple.
> > With proper color, the pictures are correct (with more green color as
> > expected for bayer).
>
> Quoting the driver's start_streaming function:
>
>         /* Set Orientation be 180 degree */
>         ret = imx258_write_reg(imx258, REG_MIRROR_FLIP_CONTROL,
>                                IMX258_REG_VALUE_08BIT, REG_CONFIG_MIRROR_FLIP);

I understand that you think it will replace the lines and columns and
the first line will be RG, instead of GB.... or actually BG because it
flips horizontal and vertical? So why does it not work?

BTW, this nicely points that the comment around
device_property_read_u32() for rotation is a little bit misleading :)

>         if (ret) {
>                 dev_err(&client->dev, "%s failed to set orientation\n",
>                         __func__);
>                 return ret;
>         }
>
> Could it be you're taking pictures of pink objects? ;-)

I can send a few sample pictures taken with GStreamer (RAW8, not
original RAW10)...

Best regards,
Krzysztof