Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201028091947.93097-1-krzk@kernel.org> <MWHPR11MB0046B799E9AD3648C6F67BFE87170@MWHPR11MB0046.namprd11.prod.outlook.com>
 <CAJKOXPePfsRNZkY+L1XM3_iz6dMYFNZAJgrcut9JriuwYkKWsw@mail.gmail.com>
 <CAJKOXPf6zhpu_3oQZ2bL_FnkBx7-NwH65N_OzVkH=Nh1bYkHxw@mail.gmail.com>
 <20201028100311.GF26150@paasikivi.fi.intel.com> <CAJKOXPcjtZidY1prH1ZCj+i-SM1mhABGbS_6_g1cH5WSGVhOAA@mail.gmail.com>
In-Reply-To: <CAJKOXPcjtZidY1prH1ZCj+i-SM1mhABGbS_6_g1cH5WSGVhOAA@mail.gmail.com>
From:   Tomasz Figa <tfiga@chromium.org>
Date:   Wed, 28 Oct 2020 13:08:28 +0100
Message-ID: <CAAFQd5AwvuuxekSdDnHNB7KiC7+mHFisEkCW71F3XQMWaFqamw@mail.gmail.com>
Subject: Re: [PATCH] media: i2c: imx258: correct mode to GBGB/RGRG
To:     Krzysztof Kozlowski <krzk@kernel.org>
Cc:     Sakari Ailus <sakari.ailus@linux.intel.com>,
        "Yeh, Andy" <andy.yeh@intel.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Jason Chen <jasonx.z.chen@intel.com>,
        Alan Chiang <alanx.chiang@intel.com>,
        "linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Oct 28, 2020 at 11:15 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> 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?

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