Linux Media Controller development
 help / color / mirror / Atom feed
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

  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