On Wed, Oct 28, 2020 at 11:15 AM Krzysztof Kozlowski <[email protected]> wrote:
>
> On Wed, 28 Oct 2020 at 11:03, Sakari Ailus <[email protected]> wrote:
> >
> > On Wed, Oct 28, 2020 at 10:56:55AM +0100, Krzysztof Kozlowski wrote:
> > > On Wed, 28 Oct 2020 at 10:45, Krzysztof Kozlowski <[email protected]> wrote:
> > > >
> > > > On Wed, 28 Oct 2020 at 10:43, Yeh, Andy <[email protected]> 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?
Any chance your SoC capture interface performs this flipping on its own as well?
>
> BTW, this nicely points that the comment around
> device_property_read_u32() for rotation is a little bit misleading :)
>
Are you referring to the comment below?
/*
* Check that the device is mounted upside down. The driver only
* supports a single pixel order right now.
*/
ret = device_property_read_u32(&client->dev, "rotation", &val);
if (ret || val != 180)
return -EINVAL;
What's misleading about it?
> > 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
On Wed, Oct 28, 2020 at 01:08:28PM +0100, Tomasz Figa wrote:
> On Wed, Oct 28, 2020 at 11:15 AM Krzysztof Kozlowski <[email protected]> wrote:
> >
> > On Wed, 28 Oct 2020 at 11:03, Sakari Ailus <[email protected]> wrote:
> > >
> > > On Wed, Oct 28, 2020 at 10:56:55AM +0100, Krzysztof Kozlowski wrote:
> > > > On Wed, 28 Oct 2020 at 10:45, Krzysztof Kozlowski <[email protected]> wrote:
> > > > >
> > > > > On Wed, 28 Oct 2020 at 10:43, Yeh, Andy <[email protected]> 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?
>
> Any chance your SoC capture interface performs this flipping on its own as well?
I messed up GStreamer as well. The color mode is correct in the driver.
>
> >
> > BTW, this nicely points that the comment around
> > device_property_read_u32() for rotation is a little bit misleading :)
> >
>
> Are you referring to the comment below?
>
> /*
> * Check that the device is mounted upside down. The driver only
> * supports a single pixel order right now.
> */
> ret = device_property_read_u32(&client->dev, "rotation", &val);
> if (ret || val != 180)
> return -EINVAL;
>
> What's misleading about it?
It's interpretation. To me the comment does not say that device
*have to* be mounted upside down. To me, it says that from all rotations
(90, -90, 180), it supports only upside down, except of course normal
mounting (no rotation).
Best regards,
Krzysztof