From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Nicola Fiorillo <nicfio@gmail.com>
Cc: "Sakari Ailus" <sakari.ailus@linux.intel.com>,
"Mauro Carvalho Chehab" <mchehab@kernel.org>,
"Hans Verkuil" <hverkuil@kernel.org>,
"Dan Scally" <dan.scally@ideasonboard.com>,
"Hans de Goede" <hansg@kernel.org>,
"Uwe Kleine-König" <u.kleine-koenig@baylibre.com>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/7] media: Add GalaxyCore GC5035 and GC8034 sensor drivers
Date: Tue, 13 Oct 2026 00:17:56 +0200 [thread overview]
Message-ID: <20261012221756.GC56180@killaraus.ideasonboard.com> (raw)
In-Reply-To: <20261012143443.34897-1-nicfio@gmail.com>
On Mon, Oct 12, 2026 at 04:34:36PM +0200, Nicola Fiorillo wrote:
> Hi all,
>
> this is v2 of the drivers for the GalaxyCore GC5035 (front camera, 2
> lanes) and GC8034 (back camera, 4 lanes) sensors of the Chuwi Hi10 X1
> tablet, with IPU6.
>
> Laurent, on v1 you asked to make the drivers configurable, with analog crop
> and binning if the sensor supports it.
>
> This version does the following:
Sorry, this cover letter is too long. Please ditch the LLM and rewrite
it manually, only including the important information. The series isn't
reviewable as-is, it gives me no confidence you're understanding what
you're doing. Same for commit messages and comments in the drivers,
please rewrite them manually.
> - No more mode tables for the geometry: the readout window and the sensor
> output crop are computed from the crop rectangle every time streaming
> starts (patches 1 and 2).
> - The crop is set with the CROP selection on the source pad (patches 4 and
> 5). Vertically it is an analog crop: the readout window gets smaller and
> the frame gets shorter (the GC5035 at 640x480 runs at up to 106 fps).
> Horizontally it is the sensor internal output crop instead: narrowing
> the readout window horizontally gives black images on the GC5035, and on
> the GC8034 it goes through a column register that gives corrupted images
> at many values.
> - 2x2 binning is there on the GC5035 (patches 6 and 7): the 1296x972
> format gives the average of four samples of the same colour, with the
> same field of view as the default crop. The vendor sequence changes the
> PLL, so I measured the link frequency in that mode: 176.3 +/- 0.9 MHz,
> against the 175.2 MHz given by the PLL registers (with the same method,
> at full resolution, 420.7 +/- 1.5 against 422.4). Patch 6 adds this
> frequency to ipu-bridge. On the GC8034 binning only exists in the 2-lane
> vendor sequences; with 4 lanes it breaks the CSI-2 stream, so it isn't
> there.
>
> I kept the crop in patches 4 and 5 and the binning in 6 and 7, separate,
> because I saw that for the IMX908 you decided to postpone the crop until
> the common raw sensor model
> (<20261001133138.GM944070@killaraus.ideasonboard.com>). If you want to do
> the same here, patches 1-3 work on their own, at a fixed resolution like
> v1, and I'll redo crop and binning on the common model when it lands.
>
> Some choices you may disagree with, and that I'm happy to change:
>
> - set_fmt changes the crop: for userspace that does not set the crop
> first, it centres it on the default crop, as ov01a10 does. And 1296x972
> selects binning and sets the default crop back, unless the crop is
> already 1296x972: libcamera first sets the largest size and then the one
> it wants, and with binning tied to the current crop it never got it. If
> you prefer binning through COMPOSE, or waiting for the common model,
> I'll change it.
> - In binning the timings are in sensor lines (the sensor reads one row of
> the array per line and outputs one every two lines): exposure and VBLANK
> keep the register unit, with a one line step, and the frame period
> computed from HBLANK and VBLANK is the measured one. This is why
> PIXEL_RATE is 70.08 MHz, HBLANK 164 and VBLANK at least 1024.
> - enum_frame_size lists 1296x972 separately, although it is within the
> full mode range, so that libcamera sees the binned mode.
> - The GC8034 PIXEL_RATE (256 MHz) is the one that gives the measured frame
> periods with the sensor line length; it is more than the 4-lane link
> carries (215 Mpixel/s), but over a whole line, blanking included, the
> average is about 196 Mpixel/s.
> - The analogue gain is the index of the sensor gain steps, which are not
> linear, as in the vendor code.
>
> I don't have the datasheets: I measured the geometry, the limits and the
> timings on the tablet, and the comments in the code say what is measured.
> In short:
>
> - The sizes reported as NATIVE_SIZE and CROP_BOUNDS (2608x1964 and
> 3282x2500) are the largest area that gives image data; around it there
> are constant rows and columns. They are not the physical array, which is
> not documented.
> - CROP_DEFAULT is the image of the vendor sequence, the same as v1: at the
> default crop v2 produces the same image as v1.
> - The timings follow a model that matches the measurements from 64 lines
> to the full height; for the GC8034 this corrected PIXEL_RATE to 256 MHz
> (v1 declared 255.9 MHz).
> - The size limits (GC8034 at least 512 and at most 3280 wide, at most 2448
> high; GC5035 at least 64x64) come from tests where outside those limits
> the stream breaks or the timings leave the model.
>
> The series is based on media next (d7e7bca71). I tested it on the
> tablet, with a 7.3-rc1 kernel and the modules built from these patches:
> - v4l2-compliance from git (1616bf9e3c81) with -z: 54/54, no warnings, on
> both, and on the GC5035 both at full resolution and in binning; full
> output below.
> - A set of crop tests (default, bounds, centre, corners, minimum, requests
> to be rounded or outside the area): rectangle applied, measured frame
> period, complete frames and position to the pixel, all as expected, in
> two runs per sensor.
> - Binning: controls, frame period (also with a long VBLANK and at the
> maximum exposure), signal doubling with double exposure, output equal to
> the per-colour 2x2 average of a full resolution frame with no offset, 20
> starts and stops in a row, and switching from the full mode to binning.
> - libcamera (simple pipeline): 60 frames out of 60 in three
> configurations per camera; on the GC5035 at 1280x960 it picks binning.
> - W=1, sparse, smatch, checkpatch --strict and coccinelle clean on every
> patch; i386 build clean.
>
> Changes since v1:
> - Patches 1 and 2: geometry removed from the register table and computed
> from the crop, all written as 16-bit values; NATIVE_SIZE and CROP_BOUNDS
> are the measured area instead of the vendor readout window; the default
> exposure no longer exceeds the maximum when VBLANK is reduced; GC8034:
> PIXEL_RATE set to 256 MHz and the blanking register computed from
> VBLANK; dropped linux/mod_devicetable.h and used a designated
> initializer in the ACPI table, as Uwe asked.
> - Added Dan's Reviewed-by to patch 3.
> - Patches 4, 5, 6 and 7: new.
>
> I used a frontier LLM (Claude) to write the code and to do the
> measurements; I stated it in the Assisted-by tags. The English of this
> letter is a translation from my Italian.
>
> Nicola
>
> v4l2-compliance (git 1616bf9e3c81, -z PCI:0000:00:05.0 -u <subdev>):
>
> GC5035 (2592x1944):
>
> v4l2-compliance 1.33.0-5515, 64 bits, 64-bit time_t
> v4l2-compliance SHA: 1616bf9e3c81 2026-10-06 07:30:20
>
> Compliance test for intel-ipu6 device /dev/v4l-subdev4:
>
> Driver Info:
> Driver version : 7.3.0
> Capabilities : 0x00000000
> Client Capabilities: 0x0000000000000002
> interval-uses-which Media Driver Info:
> Driver name : intel-ipu6
> Model : ipu6
> Serial :
> Bus info : PCI:0000:00:05.0
> Media version : 7.3.0
> Hardware revision: 0x00000000 (0)
> Driver version : 7.3.0
> Interface Info:
> ID : 0x030001fd
> Type : V4L Sub-Device
> Entity Info:
> ID : 0x000001f9 (505)
> Name : gc5035 2-003f
> Function : Camera Sensor
> Pad 0x010001fa : 0: Source
> Link 0x020001fb: to remote pad 0x1000096 of entity 'Intel IPU6 CSI2 2' (Video Interface Bridge): Data, Enabled, Immutable
>
> Required ioctls:
> test MC information (see 'Media Driver Info' above): OK
> test VIDIOC_SUDBEV_QUERYCAP: OK
> test invalid ioctls: OK
>
> Allow for multiple opens:
> test second /dev/v4l-subdev4 open: OK
> test VIDIOC_SUBDEV_QUERYCAP: OK
> test for unlimited opens: OK
>
> Debug ioctls:
> test VIDIOC_LOG_STATUS: OK (Not Supported)
>
> Input ioctls:
> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> test VIDIOC_ENUMAUDIO: OK (Not Supported)
> test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDIO: OK (Not Supported)
> Inputs: 0 Audio Inputs: 0 Tuners: 0
>
> Output ioctls:
> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> Outputs: 0 Audio Outputs: 0 Modulators: 0
>
> Input/Output configuration ioctls:
> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> test VIDIOC_G/S_EDID: OK (Not Supported)
>
> Sub-Device ioctls (Source Pad 0):
> Try Stream 0
> test Try VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> test Try VIDIOC_SUBDEV_G/S_FMT: OK
> test Try VIDIOC_SUBDEV_G/S_SELECTION/CROP: OK
> Active Stream 0
> test Active VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> test Active VIDIOC_SUBDEV_G/S_FMT: OK
> test Active VIDIOC_SUBDEV_G/S_SELECTION/CROP: OK
> test Active VIDIOC_SUBDEV_G/S_FRAME_INTERVAL: OK (Not Supported)
>
> Control ioctls:
> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
> test VIDIOC_QUERYCTRL: OK
> test VIDIOC_G/S_CTRL: OK
> test VIDIOC_G/S/TRY_EXT_CTRLS: OK
> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> Standard Controls: 13 Private Controls: 0
>
> Format ioctls:
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
> test VIDIOC_G/S_PARM: OK (Not Supported)
> test VIDIOC_G_FBUF: OK (Not Supported)
> test VIDIOC_G_FMT: OK (Not Supported)
> test VIDIOC_TRY_FMT: OK (Not Supported)
> test VIDIOC_S_FMT: OK (Not Supported)
> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> test Cropping: OK (Not Supported)
> test Composing: OK (Not Supported)
> test Scaling: OK (Not Supported)
>
> Codec ioctls:
> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>
> Buffer ioctls:
> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
> test CREATE_BUFS maximum buffers: OK
> test VIDIOC_REMOVE_BUFS: OK
> test VIDIOC_EXPBUF: OK (Not Supported)
> test Requests: OK (Not Supported)
> test blocking wait: OK (Not Supported)
>
> Total for intel-ipu6 device /dev/v4l-subdev4: 54, Succeeded: 54, Failed: 0, Warnings: 0
>
> GC5035 in binning (1296x972), same output except:
>
> Total for intel-ipu6 device /dev/v4l-subdev4: 54, Succeeded: 54, Failed: 0, Warnings: 0
>
> GC8034:
>
> v4l2-compliance 1.33.0-5515, 64 bits, 64-bit time_t
> v4l2-compliance SHA: 1616bf9e3c81 2026-10-06 07:30:20
>
> Compliance test for intel-ipu6 device /dev/v4l-subdev5:
>
> Driver Info:
> Driver version : 7.3.0
> Capabilities : 0x00000000
> Client Capabilities: 0x0000000000000002
> interval-uses-which Media Driver Info:
> Driver name : intel-ipu6
> Model : ipu6
> Serial :
> Bus info : PCI:0000:00:05.0
> Media version : 7.3.0
> Hardware revision: 0x00000000 (0)
> Driver version : 7.3.0
> Interface Info:
> ID : 0x03000203
> Type : V4L Sub-Device
> Entity Info:
> ID : 0x000001ff (511)
> Name : gc8034 1-0037
> Function : Camera Sensor
> Pad 0x01000200 : 0: Source
> Link 0x02000201: to remote pad 0x100008c of entity 'Intel IPU6 CSI2 1' (Video Interface Bridge): Data, Enabled, Immutable
>
> Required ioctls:
> test MC information (see 'Media Driver Info' above): OK
> test VIDIOC_SUDBEV_QUERYCAP: OK
> test invalid ioctls: OK
>
> Allow for multiple opens:
> test second /dev/v4l-subdev5 open: OK
> test VIDIOC_SUBDEV_QUERYCAP: OK
> test for unlimited opens: OK
>
> Debug ioctls:
> test VIDIOC_LOG_STATUS: OK (Not Supported)
>
> Input ioctls:
> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> test VIDIOC_ENUMAUDIO: OK (Not Supported)
> test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDIO: OK (Not Supported)
> Inputs: 0 Audio Inputs: 0 Tuners: 0
>
> Output ioctls:
> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> Outputs: 0 Audio Outputs: 0 Modulators: 0
>
> Input/Output configuration ioctls:
> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> test VIDIOC_G/S_EDID: OK (Not Supported)
>
> Sub-Device ioctls (Source Pad 0):
> Try Stream 0
> test Try VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> test Try VIDIOC_SUBDEV_G/S_FMT: OK
> test Try VIDIOC_SUBDEV_G/S_SELECTION/CROP: OK
> Active Stream 0
> test Active VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> test Active VIDIOC_SUBDEV_G/S_FMT: OK
> test Active VIDIOC_SUBDEV_G/S_SELECTION/CROP: OK
> test Active VIDIOC_SUBDEV_G/S_FRAME_INTERVAL: OK (Not Supported)
>
> Control ioctls:
> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
> test VIDIOC_QUERYCTRL: OK
> test VIDIOC_G/S_CTRL: OK
> test VIDIOC_G/S/TRY_EXT_CTRLS: OK
> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> Standard Controls: 12 Private Controls: 0
>
> Format ioctls:
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
> test VIDIOC_G/S_PARM: OK (Not Supported)
> test VIDIOC_G_FBUF: OK (Not Supported)
> test VIDIOC_G_FMT: OK (Not Supported)
> test VIDIOC_TRY_FMT: OK (Not Supported)
> test VIDIOC_S_FMT: OK (Not Supported)
> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> test Cropping: OK (Not Supported)
> test Composing: OK (Not Supported)
> test Scaling: OK (Not Supported)
>
> Codec ioctls:
> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>
> Buffer ioctls:
> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
> test CREATE_BUFS maximum buffers: OK
> test VIDIOC_REMOVE_BUFS: OK
> test VIDIOC_EXPBUF: OK (Not Supported)
> test Requests: OK (Not Supported)
> test blocking wait: OK (Not Supported)
>
> Total for intel-ipu6 device /dev/v4l-subdev5: 54, Succeeded: 54, Failed: 0, Warnings: 0
>
>
> Nicola Fiorillo (7):
> media: i2c: Add GC5035 image sensor driver
> media: i2c: Add GC8034 image sensor driver
> media: ipu-bridge: Add GalaxyCore GC5035 and GC8034
> media: i2c: gc5035: Add crop support
> media: i2c: gc8034: Add crop support
> media: ipu-bridge: Add the GC5035 binning link frequency
> media: i2c: gc5035: Add 2x2 binning support
>
> MAINTAINERS | 12 +
> drivers/media/i2c/Kconfig | 20 +
> drivers/media/i2c/Makefile | 2 +
> drivers/media/i2c/gc5035.c | 1351 ++++++++++++++++++++++++++
> drivers/media/i2c/gc8034.c | 1175 ++++++++++++++++++++++
> drivers/media/pci/intel/ipu-bridge.c | 4 +
> 6 files changed, 2564 insertions(+)
> create mode 100644 drivers/media/i2c/gc5035.c
> create mode 100644 drivers/media/i2c/gc8034.c
>
>
> base-commit: d7e7bca7169873b46e857c025c10f3336c1258d8
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2026-10-12 22:17 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-09 7:27 [PATCH 0/3] media: Add GalaxyCore GC5035 and GC8034 sensor drivers Nicola Fiorillo
2026-10-09 7:27 ` [PATCH 1/3] media: i2c: Add GC5035 image sensor driver Nicola Fiorillo
2026-10-09 9:48 ` Laurent Pinchart
2026-10-12 11:20 ` Uwe Kleine-König
2026-10-09 7:27 ` [PATCH 2/3] media: i2c: Add GC8034 " Nicola Fiorillo
2026-10-09 7:27 ` [PATCH 3/3] media: ipu-bridge: Add GalaxyCore GC5035 and GC8034 Nicola Fiorillo
2026-10-09 9:24 ` Dan Scally
2026-10-12 14:34 ` [PATCH v2 0/7] media: Add GalaxyCore GC5035 and GC8034 sensor drivers Nicola Fiorillo
2026-10-12 14:34 ` [PATCH v2 1/7] media: i2c: Add GC5035 image sensor driver Nicola Fiorillo
2026-10-12 14:34 ` [PATCH v2 2/7] media: i2c: Add GC8034 " Nicola Fiorillo
2026-10-12 14:34 ` [PATCH v2 3/7] media: ipu-bridge: Add GalaxyCore GC5035 and GC8034 Nicola Fiorillo
2026-10-12 14:34 ` [PATCH v2 4/7] media: i2c: gc5035: Add crop support Nicola Fiorillo
2026-10-12 14:34 ` [PATCH v2 5/7] media: i2c: gc8034: " Nicola Fiorillo
2026-10-12 14:34 ` [PATCH v2 6/7] media: ipu-bridge: Add the GC5035 binning link frequency Nicola Fiorillo
2026-10-12 14:34 ` [PATCH v2 7/7] media: i2c: gc5035: Add 2x2 binning support Nicola Fiorillo
2026-10-12 22:17 ` Laurent Pinchart [this message]
-- strict thread matches above, loose matches on Subject: below --
2026-10-13 4:08 [PATCH v2 0/7] media: Add GalaxyCore GC5035 and GC8034 sensor drivers Nicola Fiorillo
2026-10-13 6:01 ` Laurent Pinchart
2026-10-13 13:56 ` Nicola Fiorillo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261012221756.GC56180@killaraus.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=dan.scally@ideasonboard.com \
--cc=hansg@kernel.org \
--cc=hverkuil@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=nicfio@gmail.com \
--cc=sakari.ailus@linux.intel.com \
--cc=u.kleine-koenig@baylibre.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox