Hi,
On Tue, Oct 31, 2023 at 09:05:32AM +0100, Jacopo Mondi wrote:
> Hi Alain
>
> On Mon, Oct 30, 2023 at 05:37:11PM +0100, Alain Volmat wrote:
> > Hi Jacopo,
> >
> > On Mon, Oct 23, 2023 at 10:38:59AM +0200, Jacopo Mondi wrote:
> > > Hi Alain
> > >
> > > On Wed, Oct 11, 2023 at 07:57:30PM +0200, Alain Volmat wrote:
> > > > Addition of support for the Galaxy Core GC2145 XVGA sensor.
> > > > The sensor supports both DVP and CSI-2 interfaces however for
> > > > the time being only CSI-2 is implemented.
> > > >
> > > > Configurations is currently based on initialization scripts
> > > > coming from Galaxy Core and for that purpose only 3 static
> > > > resolutions are supported with static framerates.
> > > > - 640x480 (30fps)
> > > > - 1280x720 (30fps)
> > > > - 1600x1200 (20fps)
> > >
> > > Anything blocking having a writable VBLANK ? This is a YUV sensor but
> > > GC2145_REG_VBLANK seems to be writable. I don't want to push you to
> > > more work that what you actually need, but configurable blankings (and
> > > then frame durations) seems like an important feature ? (and if I
> > > recall right you want to use this sensor with libcamera, which will
> > > require blankings to be controllable (if the sensor supports any RAW
> > > format)
> >
> > No, nothing prevents to write the VBLANK register. I just did some
> > tests directly via rwmem into a running sensor and vertical blanking can
> > be updated, allowing to tune the framerate.
> >
> > >
> > > I don't see any RAW format being supported in this version. Is this
> > > something you plan to do on top ?
> >
> > Yes, absolutely, it is possible to output RAW formats as well however
> > this version of the driver doesn't support it yet. The plan is indeed
> > to add it on top of this.
> > Several things to be addressed on top of this serie:
> > - RAW format
> > - frame_interval vs H/V blank control. Is my understanding correct if
> > I say that if a sensor has RAW format (even if it ALSO has YUV /
> > RGB) then control is done via H/V blanking controls rather than
> > frame_interval ?
>
> I'll reply here to this question that is asked in a few other places.
>
> I can only point you to the ov5640 driver, which is capable of both
> YUV/RGB and RAW as this sensor is. The ov5640 driver supports both the
> g/s/enum_frame_interval and has writable blankings. I guess it's more
> for historical reasons, as when blankings have been made writable
> users of the frame_interval API would have been displeased if that
> interface went away.
>
> The resulting implementation is not nice, as changing vblank doesn't
> update the framerate reported through g_frame_interval, and keeping
> the two in sync is not trivial.
>
> I would suggest to go for writable blankings, and if you already plan
> to remove frame_interval then not add it in first place so there won't
> be displeased users.
>
> Sakari, Laurent, what's your opinion here ?
Sakari, Laurent, do you have opinion regarding how to control the
framerate on this sensor. It is a YUV sensor but which might also (not
yet made available in this first serie) support RAW format.
Should I expose the g_frame_interval / s_frame_interval or only blanking
ctrls ?
>
>
> > - parallel interface support
> >
> > >
> > > >
>
> [snip]
>
> > > > +/**
> > > > + * struct gc2145_format - GC2145 pixel format description
> > > > + * @code: media bus (MBUS) associated code
> > > > + * @colorspace: V4L2 colospace
> > > > + * @datatype: MIPI CSI2 data type
> > > > + * @output_fmt: GC2145 output format
> > > > + */
> > > > +struct gc2145_format {
> > > > + unsigned int code;
> > > > + unsigned int colorspace;
> > > > + unsigned char datatype;
> > > > + unsigned char output_fmt;
> > > > +};
> > > > +
> > > > +/* All supported formats */
> > > > +static const struct gc2145_format supported_formats[] = {
> > > > + {
> > > > + .code = MEDIA_BUS_FMT_UYVY8_2X8,
> > >
> > > The driver supports CSI-2, the 1X16 format variants should be used for
> > > serial bus
> >
> > Yes. Doing this, this actually triggered big questioning since it seems
> > that the sensor, even in CSI, seems to be sending the RGB565 in
> > big-endian format ;-( I have just sent an email to GalaxyCore to clarify
> > this point however, once captured, I need to swap bytes in order to get
> > the right colors ;-( This is the reason why I used the RGB565_2X8_BE in
> > the first place and I believe this is correct for parallel mode, but my
> > understanding of the CSI formats makes me think that the sensor should
> > be sending the data differently.
> > I will wait for GalaxyCore reply before sending the v3.
> >
>
> Let's wait for GC to get back with more information then, it still
> feels weird that a CSI-2 compliant sensor sends data out in a way
> different from what is described in the spec..
Format fixed in v3, got reply from GC, there were a bit in order to allow
switching the first/second cycle (if referring to parallel way to
transmit). With that done, MIPI transmitted data are in LE.
Regards,
Alain
