* [PATCH v1 0/4] media: Add Sony IMX585 image sensor support
@ 2025-07-02 6:38 Will Whang
2025-07-02 6:38 ` [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor Will Whang
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Will Whang @ 2025-07-02 6:38 UTC (permalink / raw)
To: Will Whang, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
The following 4-patch series adds upstream support for the Sony
IMX585 CMOS image-sensor.
Features
==========
* 4-lane or 2-lane MIPI-CSI-2 up to 2079 Mbps/lane
* 4 K @ 60 fps 12-bit linear, 4 K @ 30 fps 16-bit Clear-HDR,
4 K @ 30 fps 12-bit gradient compression Clear-HDR
1080p binning mode, mono hardware variant, HCG support.
* New Sensor Dependent V4L2 controls for
HDR blending, grad-compression, HCG enable and Enable ClearHDR.
* Additional IRCut filter support through V4L2_CID_BAND_STOP_FILTER.
* Blacklevel adjustments through V4L2_CID_BRIGHTNESS.
* Multi Camera synchronization mode support.
The driver has been validated on Raspberry Pi 4 B and Pi 5 using libcamera
with a 24 MHz master clock.
Series layout
=============
1. **dt-bindings: media: Add Sony IMX585 CMOS image sensor**
2. **media: uapi: Add custom IMX585 control IDs**
3. **media: i2c: imx585: Add Sony IMX585 image-sensor driver**
4. **media: docs: Add userspace-API guide for the IMX585 driver**
Feedback is welcome, in particular on the private-control API.
Thanks in advance for your review!
v1:
- initial posting
Regards,
Will
Will Whang (4):
dt-bindings: media: Add Sony IMX585 CMOS image sensor
media: uapi: Add custom IMX585 control IDs
media: i2c: imx585: Add Sony IMX585 image-sensor driver
media: docs: Add userspace-API guide for the IMX585 driver
.../bindings/media/i2c/sony,imx585.yaml | 120 +
.../userspace-api/media/drivers/imx585.rst | 95 +
MAINTAINERS | 8 +
drivers/media/i2c/Kconfig | 9 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/imx585.c | 2466 +++++++++++++++++
include/uapi/linux/imx585.h | 20 +
include/uapi/linux/v4l2-controls.h | 6 +
8 files changed, 2725 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
create mode 100644 Documentation/userspace-api/media/drivers/imx585.rst
create mode 100644 drivers/media/i2c/imx585.c
create mode 100644 include/uapi/linux/imx585.h
--
2.39.5
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
2025-07-02 6:38 [PATCH v1 0/4] media: Add Sony IMX585 image sensor support Will Whang
@ 2025-07-02 6:38 ` Will Whang
2025-07-02 9:28 ` Laurent Pinchart
2025-07-04 8:08 ` Krzysztof Kozlowski
2025-07-02 6:38 ` [PATCH v1 2/4] media: uapi: Add custom IMX585 control IDs Will Whang
` (2 subsequent siblings)
3 siblings, 2 replies; 12+ messages in thread
From: Will Whang @ 2025-07-02 6:38 UTC (permalink / raw)
To: Will Whang, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Document the devicetree binding for the Sony IMX585. The schema
covers the CSI-2 data-lanes, the optional 'mono-mode' flag,
and the internal-sync properties used by the driver.
Signed-off-by: Will Whang <will@willwhang.com>
---
.../bindings/media/i2c/sony,imx585.yaml | 120 ++++++++++++++++++
MAINTAINERS | 8 ++
2 files changed, 128 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
new file mode 100644
index 000000000..d050d1642
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2024 Ideas on Board Oy
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/sony,imx585.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony IMX585 Sensor
+
+maintainers:
+ - Will Whang <will@willwhang.com>
+
+description:
+ IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
+
+properties:
+ compatible:
+ const: sony,imx585
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ const: xclk
+
+ clock-frequency:
+ enum: [ 74250000, 37125000, 72000000, 27000000, 24000000 ]
+
+ reg:
+ maxItems: 1
+ description: I2C Address for IMX585
+
+ VANA-supply:
+ description: Analog power supply (3.3V)
+
+ VDDL-supply:
+ description: Interface power supply (1.8V)
+
+ VDIG-supply:
+ description: Digital power supply (1.1V)
+
+ reset-gpios:
+ description: Sensor reset (XCLR) GPIO
+ maxItems: 1
+
+ port:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ additionalProperties: false
+
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ data-lanes:
+ anyOf:
+ - items:
+ - const: 1
+ - const: 2
+ - const: 3
+ - const: 4
+
+ sync-mode:
+ description: |
+ Select the synchronisation mode of the sensor
+ 0 – internal sync, leader (default)
+ 1 – internal sync, follower
+ 2 – external sync
+ $ref: /schemas/types.yaml#/definitions/uint8
+ enum: [ 0, 1, 2 ]
+
+ link-frequencies:
+ description: Select the MIPI-CSI2 link speed in Mhz
+ items:
+ enum: [ 297000000, 360000000, 445500000, 594000000,
+ 720000000, 891000000, 1039500000 ]
+
+ required:
+ - data-lanes
+ - link-frequencies
+
+ required:
+ - endpoint
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-frequency
+ - port
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ imx585@1a {
+ compatible = "sony,imx585";
+ reg = <0x1a>;
+ clocks = <&imx585_clk>;
+ clock-frequency = <24000000>;
+
+ VANA-supply = <&camera_vadd_3v3>;
+ VDDL-supply = <&camera_vdd1_1v8>;
+ VDIG-supply = <&camera_vdd2_1v1>;
+
+ port {
+ imx585: endpoint {
+ remote-endpoint = <&cam>;
+ data-lanes = <1 2 3 4>;
+ link-frequencies = /bits/ 64 <720000000>;
+ };
+ };
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index da34c7227..9cc404790 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23150,6 +23150,14 @@ T: git git://linuxtv.org/media.git
F: Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
F: drivers/media/i2c/imx415.c
+SONY IMX585 SENSOR DRIVER
+M: Will Whang <will@willwhang.com>
+L: linux-media@vger.kernel.org
+S: Maintained
+T: git git://linuxtv.org/media.git
+F: Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
+F: drivers/media/i2c/imx585.c
+
SONY MEMORYSTICK SUBSYSTEM
M: Maxim Levitsky <maximlevitsky@gmail.com>
M: Alex Dubov <oakad@yahoo.com>
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 2/4] media: uapi: Add custom IMX585 control IDs
2025-07-02 6:38 [PATCH v1 0/4] media: Add Sony IMX585 image sensor support Will Whang
2025-07-02 6:38 ` [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor Will Whang
@ 2025-07-02 6:38 ` Will Whang
2025-07-02 6:38 ` [PATCH v1 3/4] media: i2c: imx585: Add Sony IMX585 image-sensor driver Will Whang
2025-07-02 6:38 ` [PATCH v1 4/4] media: docs: Add userspace-API guide for the IMX585 driver Will Whang
3 siblings, 0 replies; 12+ messages in thread
From: Will Whang @ 2025-07-02 6:38 UTC (permalink / raw)
To: Will Whang, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Reserve a private control ID block and define the helper enums
used by the upcoming IMX585 driver.
Signed-off-by: Will Whang <will@willwhang.com>
---
include/uapi/linux/imx585.h | 20 ++++++++++++++++++++
include/uapi/linux/v4l2-controls.h | 6 ++++++
2 files changed, 26 insertions(+)
create mode 100644 include/uapi/linux/imx585.h
diff --git a/include/uapi/linux/imx585.h b/include/uapi/linux/imx585.h
new file mode 100644
index 000000000..30e12df88
--- /dev/null
+++ b/include/uapi/linux/imx585.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * IMX585 V4L2 controls user space header file.
+ *
+ */
+
+#ifndef __UAPI_IMX585_H_
+#define __UAPI_IMX585_H_
+
+#include <linux/v4l2-controls.h>
+
+#define V4L2_CID_IMX585_HDR_DATASEL_TH (V4L2_CID_USER_IMX585_BASE + 0)
+#define V4L2_CID_IMX585_HDR_DATASEL_BK (V4L2_CID_USER_IMX585_BASE + 1)
+#define V4L2_CID_IMX585_HDR_GRAD_TH (V4L2_CID_USER_IMX585_BASE + 2)
+#define V4L2_CID_IMX585_HDR_GRAD_COMP_L (V4L2_CID_USER_IMX585_BASE + 3)
+#define V4L2_CID_IMX585_HDR_GRAD_COMP_H (V4L2_CID_USER_IMX585_BASE + 4)
+#define V4L2_CID_IMX585_HDR_GAIN (V4L2_CID_USER_IMX585_BASE + 5)
+#define V4L2_CID_IMX585_HCG_GAIN (V4L2_CID_USER_IMX585_BASE + 6)
+
+#endif /* __UAPI_IMX585_H_ */
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index f836512e9..091a044e5 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -228,6 +228,12 @@ enum v4l2_colorfx {
*/
#define V4L2_CID_USER_RKISP1_BASE (V4L2_CID_USER_BASE + 0x1220)
+/*
+ * The base for IMX585 driver controls.
+ * We reserve 16 controls for this driver.
+ */
+#define V4L2_CID_USER_IMX585_BASE (V4L2_CID_USER_BASE + 0x1230)
+
/* MPEG-class control IDs */
/* The MPEG controls are applicable to all codec controls
* and the 'MPEG' part of the define is historical */
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 3/4] media: i2c: imx585: Add Sony IMX585 image-sensor driver
2025-07-02 6:38 [PATCH v1 0/4] media: Add Sony IMX585 image sensor support Will Whang
2025-07-02 6:38 ` [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor Will Whang
2025-07-02 6:38 ` [PATCH v1 2/4] media: uapi: Add custom IMX585 control IDs Will Whang
@ 2025-07-02 6:38 ` Will Whang
2025-07-03 17:51 ` Laurent Pinchart
2025-07-02 6:38 ` [PATCH v1 4/4] media: docs: Add userspace-API guide for the IMX585 driver Will Whang
3 siblings, 1 reply; 12+ messages in thread
From: Will Whang @ 2025-07-02 6:38 UTC (permalink / raw)
To: Will Whang, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Implements support for:
* 4-lane / 2-lane CSI-2
* 12-bit linear, 12-bit HDR-GC and 16-bit Clear-HDR modes
* Mono variant switch, HCG, custom HDR controls
* Tested on Raspberry Pi 4/5 with 24 MHz XCLK.
Signed-off-by: Will Whang <will@willwhang.com>
---
drivers/media/i2c/Kconfig | 9 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/imx585.c | 2466 ++++++++++++++++++++++++++++++++++++
3 files changed, 2476 insertions(+)
create mode 100644 drivers/media/i2c/imx585.c
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index e68202954..34eb1c19a 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -266,6 +266,15 @@ config VIDEO_IMX415
To compile this driver as a module, choose M here: the
module will be called imx415.
+config VIDEO_IMX585
+ tristate "Sony IMX585 sensor support"
+ help
+ This is a Video4Linux2 sensor driver for the Sony
+ IMX585 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx585.
+
config VIDEO_MAX9271_LIB
tristate
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 5873d2943..887d19ca7 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_VIDEO_IMX335) += imx335.o
obj-$(CONFIG_VIDEO_IMX355) += imx355.o
obj-$(CONFIG_VIDEO_IMX412) += imx412.o
obj-$(CONFIG_VIDEO_IMX415) += imx415.o
+obj-$(CONFIG_VIDEO_IMX585) += imx585.o
obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
obj-$(CONFIG_VIDEO_ISL7998X) += isl7998x.o
obj-$(CONFIG_VIDEO_KS0127) += ks0127.o
diff --git a/drivers/media/i2c/imx585.c b/drivers/media/i2c/imx585.c
new file mode 100644
index 000000000..2c4212290
--- /dev/null
+++ b/drivers/media/i2c/imx585.c
@@ -0,0 +1,2466 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A V4L2 driver for Sony imx585 cameras.
+ *
+ * Based on Sony imx477 camera driver
+ * Copyright (C) 2019-2020 Raspberry Pi (Trading) Ltd
+ * Modified by Will WHANG
+ * Modified by sohonomura2020 in Soho Enterprise Ltd.
+ */
+#include <linux/unaligned.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mediabus.h>
+
+// Support for rpi kernel pre git commit 314a685
+#ifndef MEDIA_BUS_FMT_SENSOR_DATA
+#define MEDIA_BUS_FMT_SENSOR_DATA 0x7002
+#endif
+
+#define V4L2_CID_IMX585_HDR_DATASEL_TH (V4L2_CID_USER_IMX585_BASE + 0)
+#define V4L2_CID_IMX585_HDR_DATASEL_BK (V4L2_CID_USER_IMX585_BASE + 1)
+#define V4L2_CID_IMX585_HDR_GRAD_TH (V4L2_CID_USER_IMX585_BASE + 2)
+#define V4L2_CID_IMX585_HDR_GRAD_COMP_L (V4L2_CID_USER_IMX585_BASE + 3)
+#define V4L2_CID_IMX585_HDR_GRAD_COMP_H (V4L2_CID_USER_IMX585_BASE + 4)
+#define V4L2_CID_IMX585_HDR_GAIN (V4L2_CID_USER_IMX585_BASE + 5)
+#define V4L2_CID_IMX585_HCG_GAIN (V4L2_CID_USER_IMX585_BASE + 6)
+
+/* Standby or streaming mode */
+#define IMX585_REG_MODE_SELECT 0x3000
+#define IMX585_MODE_STANDBY 0x01
+#define IMX585_MODE_STREAMING 0x00
+#define IMX585_STREAM_DELAY_US 25000
+#define IMX585_STREAM_DELAY_RANGE_US 1000
+
+/*
+ * Initialisation delay between XCLR low->high and the moment when the sensor
+ * can start capture (i.e. can leave software standby)
+ */
+#define IMX585_XCLR_MIN_DELAY_US 500000
+#define IMX585_XCLR_DELAY_RANGE_US 1000
+
+/* Leader mode and XVS/XHS direction */
+#define IMX585_REG_XMSTA 0x3002
+#define IMX585_REG_XXS_DRV 0x30A6
+#define IMX585_REG_EXTMODE 0x30CE
+#define IMX585_REG_XXS_OUTSEL 0x30A4
+
+/*XVS pulse length, 2^n H with n=0~3*/
+#define IMX585_REG_XVSLNG 0x30CC
+/*XHS pulse length, 16*(2^n) Clock with n=0~3*/
+#define IMX585_REG_XHSLNG 0x30CD
+
+/* Clk selection */
+#define IMX585_INCK_SEL 0x3014
+
+/* Link Speed */
+#define IMX585_DATARATE_SEL 0x3015
+
+/* BIN mode */
+/* 2x2 Bin mode selection, 0x01 => Mono, 0x00 => Color */
+#define IMX585_BIN_MODE 0x3019
+
+/* Lane Count */
+#define IMX585_LANEMODE 0x3040
+
+/* VMAX internal VBLANK*/
+#define IMX585_REG_VMAX 0x3028
+#define IMX585_VMAX_MAX 0xfffff
+#define IMX585_VMAX_DEFAULT 2250
+
+/* HMAX internal HBLANK*/
+#define IMX585_REG_HMAX 0x302C
+#define IMX585_HMAX_MAX 0xffff
+
+/* SHR internal */
+#define IMX585_REG_SHR 0x3050
+#define IMX585_SHR_MIN 8
+#define IMX585_SHR_MIN_HDR 10
+#define IMX585_SHR_MAX 0xfffff
+
+/* Exposure control */
+#define IMX585_EXPOSURE_MIN 2
+#define IMX585_EXPOSURE_STEP 1
+#define IMX585_EXPOSURE_DEFAULT 1000
+#define IMX585_EXPOSURE_MAX 49865
+
+/* HDR threshold */
+#define IMX585_REG_EXP_TH_H 0x36D0
+#define IMX585_REG_EXP_TH_L 0x36D4
+#define IMX585_REG_EXP_BK 0x36E2
+
+/* Gradation compression control */
+#define IMX595_REG_CCMP_EN 0x36EF
+#define IMX585_REG_CCMP1_EXP 0x36E8
+#define IMX585_REG_CCMP2_EXP 0x36E4
+#define IMX585_REG_ACMP1_EXP 0x36EE
+#define IMX585_REG_ACMP2_EXP 0x36EC
+
+/* HDR Gain Adder */
+#define IMX585_REG_EXP_GAIN 0x3081
+
+/* Black level control */
+#define IMX585_REG_BLKLEVEL 0x30DC
+#define IMX585_BLKLEVEL_DEFAULT 50
+
+/* Digital Clamp */
+#define IMX585_REG_DIGITAL_CLAMP 0x3458
+
+/* Analog gain control */
+#define IMX585_REG_ANALOG_GAIN 0x306C
+#define IMX585_REG_FDG_SEL0 0x3030
+#define IMX585_ANA_GAIN_MIN_NORMAL 0
+#define IMX585_ANA_GAIN_MIN_HCG 34
+#define IMX585_ANA_GAIN_MAX_HDR 80
+#define IMX585_ANA_GAIN_MAX_NORMAL 240
+#define IMX585_ANA_GAIN_STEP 1
+#define IMX585_ANA_GAIN_DEFAULT 0
+
+/* Flip */
+#define IMX585_FLIP_WINMODEH 0x3020
+#define IMX585_FLIP_WINMODEV 0x3021
+
+/* Embedded metadata stream structure */
+#define IMX585_EMBEDDED_LINE_WIDTH 16384
+#define IMX585_NUM_EMBEDDED_LINES 1
+
+#define IMX585_PIXEL_RATE 74250000
+
+enum pad_types {
+ IMAGE_PAD,
+ METADATA_PAD,
+ NUM_PADS
+};
+
+/* imx585 native and active pixel array size. */
+#define IMX585_NATIVE_WIDTH 3856U
+#define IMX585_NATIVE_HEIGHT 2180U
+#define IMX585_PIXEL_ARRAY_LEFT 8U
+#define IMX585_PIXEL_ARRAY_TOP 8U
+#define IMX585_PIXEL_ARRAY_WIDTH 3840U
+#define IMX585_PIXEL_ARRAY_HEIGHT 2160U
+
+/* Link frequency setup */
+enum {
+ IMX585_LINK_FREQ_297MHZ, // 594Mbps/lane
+ IMX585_LINK_FREQ_360MHZ, // 720Mbps/lane
+ IMX585_LINK_FREQ_445MHZ, // 891Mbps/lane
+ IMX585_LINK_FREQ_594MHZ, // 1188Mbps/lane
+ IMX585_LINK_FREQ_720MHZ, // 1440Mbps/lane
+ IMX585_LINK_FREQ_891MHZ, // 1782Mbps/lane
+ IMX585_LINK_FREQ_1039MHZ, // 2079Mbps/lane
+ IMX585_LINK_FREQ_1188MHZ, // 2376Mbps/lane
+};
+
+static const u8 link_freqs_reg_value[] = {
+ [IMX585_LINK_FREQ_297MHZ] = 0x07,
+ [IMX585_LINK_FREQ_360MHZ] = 0x06,
+ [IMX585_LINK_FREQ_445MHZ] = 0x05,
+ [IMX585_LINK_FREQ_594MHZ] = 0x04,
+ [IMX585_LINK_FREQ_720MHZ] = 0x03,
+ [IMX585_LINK_FREQ_891MHZ] = 0x02,
+ [IMX585_LINK_FREQ_1039MHZ] = 0x01,
+ [IMX585_LINK_FREQ_1188MHZ] = 0x00,
+};
+
+static const u64 link_freqs[] = {
+ [IMX585_LINK_FREQ_297MHZ] = 297000000,
+ [IMX585_LINK_FREQ_360MHZ] = 360000000,
+ [IMX585_LINK_FREQ_445MHZ] = 445500000,
+ [IMX585_LINK_FREQ_594MHZ] = 594000000,
+ [IMX585_LINK_FREQ_720MHZ] = 720000000,
+ [IMX585_LINK_FREQ_891MHZ] = 891000000,
+ [IMX585_LINK_FREQ_1039MHZ] = 1039500000,
+ [IMX585_LINK_FREQ_1188MHZ] = 1188000000,
+};
+
+//min HMAX for 4-lane 4K full res mode, x2 for 2-lane, /2 for FHD
+static const u16 HMAX_table_4lane_4K[] = {
+ [IMX585_LINK_FREQ_297MHZ] = 1584,
+ [IMX585_LINK_FREQ_360MHZ] = 1320,
+ [IMX585_LINK_FREQ_445MHZ] = 1100,
+ [IMX585_LINK_FREQ_594MHZ] = 792,
+ [IMX585_LINK_FREQ_720MHZ] = 660,
+ [IMX585_LINK_FREQ_891MHZ] = 550,
+ [IMX585_LINK_FREQ_1039MHZ] = 440,
+ [IMX585_LINK_FREQ_1188MHZ] = 396,
+};
+
+struct imx585_inck_cfg {
+ u32 xclk_hz; /* platform clock rate */
+ u8 inck_sel; /* value for reg */
+};
+
+static const struct imx585_inck_cfg imx585_inck_table[] = {
+ { 74250000, 0x00 },
+ { 37125000, 0x01 },
+ { 72000000, 0x02 },
+ { 27000000, 0x03 },
+ { 24000000, 0x04 },
+};
+
+static const char * const hdr_gain_adder_menu[] = {
+ "+0dB",
+ "+6dB",
+ "+12dB",
+ "+18dB",
+ "+24dB",
+ "+29.1dB",
+};
+
+/*Honestly I don't know why there are two 50% 50% blend
+ * but it is in the datasheet
+ */
+static const char * const hdr_data_blender_menu[] = {
+ "HG 1/2, LG 1/2",
+ "HG 3/4, LG 1/4",
+ "HG 1/2, LG 1/2",
+ "HG 7/8, LG 1/8",
+ "HG 15/16, LG 1/16",
+ "2nd HG 1/2, LG 1/2",
+ "HG 1/16, LG 15/16",
+ "HG 1/8, LG 7/8",
+ "HG 1/4, LG 3/4",
+};
+
+static const char * const grad_compression_slope_menu[] = {
+ "1/1",
+ "1/2",
+ "1/4",
+ "1/8",
+ "1/16",
+ "1/32",
+ "1/64",
+ "1/128",
+ "1/256",
+ "1/512",
+ "1/1024",
+ "1/2048",
+};
+
+static const char * const sync_mode_menu[] = {
+ "Internal Sync Leader Mode",
+ "External Sync Leader Mode",
+ "Follower Mode",
+};
+
+struct imx585_reg {
+ u16 address;
+ u8 val;
+};
+
+struct IMX585_reg_list {
+ unsigned int num_of_regs;
+ const struct imx585_reg *regs;
+};
+
+/* Mode : resolution and related config&values */
+struct imx585_mode {
+ /* Frame width */
+ unsigned int width;
+
+ /* Frame height */
+ unsigned int height;
+
+ /* mode HMAX Scaling */
+ u8 hmax_div;
+
+ /* minimum H-timing */
+ u16 min_HMAX;
+
+ /* minimum V-timing */
+ u64 min_VMAX;
+
+ /* default H-timing */
+ u16 default_HMAX;
+
+ /* default V-timing */
+ u64 default_VMAX;
+
+ /* Analog crop rectangle. */
+ struct v4l2_rect crop;
+
+ /* Default register values */
+ struct IMX585_reg_list reg_list;
+};
+
+/* IMX585 Register List */
+/* Common Modes */
+static struct imx585_reg common_regs[] = {
+ {0x3002, 0x01},
+ {0x3069, 0x00},
+ {0x3074, 0x64},
+ {0x30D5, 0x04},// DIG_CLP_VSTART
+ {0x3030, 0x00},// FDG_SEL0 LCG, HCG:0x01
+ {0x30A6, 0x00},// XVS_DRV [1:0] Hi-Z
+ {0x3081, 0x00},// EXP_GAIN, Reset to 0
+ {0x3460, 0x21},// -
+ {0x3478, 0xA1},// -
+ {0x347C, 0x01},// -
+ {0x3480, 0x01},// -
+ {0x3A4E, 0x14},// -
+ {0x3A52, 0x14},// -
+ {0x3A56, 0x00},// -
+ {0x3A5A, 0x00},// -
+ {0x3A5E, 0x00},// -
+ {0x3A62, 0x00},// -
+ {0x3A6A, 0x20},// -
+ {0x3A6C, 0x42},// -
+ {0x3A6E, 0xA0},// -
+ {0x3B2C, 0x0C},// -
+ {0x3B30, 0x1C},// -
+ {0x3B34, 0x0C},// -
+ {0x3B38, 0x1C},// -
+ {0x3BA0, 0x0C},// -
+ {0x3BA4, 0x1C},// -
+ {0x3BA8, 0x0C},// -
+ {0x3BAC, 0x1C},// -
+ {0x3D3C, 0x11},// -
+ {0x3D46, 0x0B},// -
+ {0x3DE0, 0x3F},// -
+ {0x3DE1, 0x08},// -
+ {0x3E14, 0x87},// -
+ {0x3E16, 0x91},// -
+ {0x3E18, 0x91},// -
+ {0x3E1A, 0x87},// -
+ {0x3E1C, 0x78},// -
+ {0x3E1E, 0x50},// -
+ {0x3E20, 0x50},// -
+ {0x3E22, 0x50},// -
+ {0x3E24, 0x87},// -
+ {0x3E26, 0x91},// -
+ {0x3E28, 0x91},// -
+ {0x3E2A, 0x87},// -
+ {0x3E2C, 0x78},// -
+ {0x3E2E, 0x50},// -
+ {0x3E30, 0x50},// -
+ {0x3E32, 0x50},// -
+ {0x3E34, 0x87},// -
+ {0x3E36, 0x91},// -
+ {0x3E38, 0x91},// -
+ {0x3E3A, 0x87},// -
+ {0x3E3C, 0x78},// -
+ {0x3E3E, 0x50},// -
+ {0x3E40, 0x50},// -
+ {0x3E42, 0x50},// -
+ {0x4054, 0x64},// -
+ {0x4148, 0xFE},// -
+ {0x4149, 0x05},// -
+ {0x414A, 0xFF},// -
+ {0x414B, 0x05},// -
+ {0x420A, 0x03},// -
+ {0x4231, 0x08},// -
+ {0x423D, 0x9C},// -
+ {0x4242, 0xB4},// -
+ {0x4246, 0xB4},// -
+ {0x424E, 0xB4},// -
+ {0x425C, 0xB4},// -
+ {0x425E, 0xB6},// -
+ {0x426C, 0xB4},// -
+ {0x426E, 0xB6},// -
+ {0x428C, 0xB4},// -
+ {0x428E, 0xB6},// -
+ {0x4708, 0x00},// -
+ {0x4709, 0x00},// -
+ {0x470A, 0xFF},// -
+ {0x470B, 0x03},// -
+ {0x470C, 0x00},// -
+ {0x470D, 0x00},// -
+ {0x470E, 0xFF},// -
+ {0x470F, 0x03},// -
+ {0x47EB, 0x1C},// -
+ {0x47F0, 0xA6},// -
+ {0x47F2, 0xA6},// -
+ {0x47F4, 0xA0},// -
+ {0x47F6, 0x96},// -
+ {0x4808, 0xA6},// -
+ {0x480A, 0xA6},// -
+ {0x480C, 0xA0},// -
+ {0x480E, 0x96},// -
+ {0x492C, 0xB2},// -
+ {0x4930, 0x03},// -
+ {0x4932, 0x03},// -
+ {0x4936, 0x5B},// -
+ {0x4938, 0x82},// -
+ {0x493E, 0x23},// -
+ {0x4BA8, 0x1C},// -
+ {0x4BA9, 0x03},// -
+ {0x4BAC, 0x1C},// -
+ {0x4BAD, 0x1C},// -
+ {0x4BAE, 0x1C},// -
+ {0x4BAF, 0x1C},// -
+ {0x4BB0, 0x1C},// -
+ {0x4BB1, 0x1C},// -
+ {0x4BB2, 0x1C},// -
+ {0x4BB3, 0x1C},// -
+ {0x4BB4, 0x1C},// -
+ {0x4BB8, 0x03},// -
+ {0x4BB9, 0x03},// -
+ {0x4BBA, 0x03},// -
+ {0x4BBB, 0x03},// -
+ {0x4BBC, 0x03},// -
+ {0x4BBD, 0x03},// -
+ {0x4BBE, 0x03},// -
+ {0x4BBF, 0x03},// -
+ {0x4BC0, 0x03},// -
+ {0x4C14, 0x87},// -
+ {0x4C16, 0x91},// -
+ {0x4C18, 0x91},// -
+ {0x4C1A, 0x87},// -
+ {0x4C1C, 0x78},// -
+ {0x4C1E, 0x50},// -
+ {0x4C20, 0x50},// -
+ {0x4C22, 0x50},// -
+ {0x4C24, 0x87},// -
+ {0x4C26, 0x91},// -
+ {0x4C28, 0x91},// -
+ {0x4C2A, 0x87},// -
+ {0x4C2C, 0x78},// -
+ {0x4C2E, 0x50},// -
+ {0x4C30, 0x50},// -
+ {0x4C32, 0x50},// -
+ {0x4C34, 0x87},// -
+ {0x4C36, 0x91},// -
+ {0x4C38, 0x91},// -
+ {0x4C3A, 0x87},// -
+ {0x4C3C, 0x78},// -
+ {0x4C3E, 0x50},// -
+ {0x4C40, 0x50},// -
+ {0x4C42, 0x50},// -
+ {0x4D12, 0x1F},// -
+ {0x4D13, 0x1E},// -
+ {0x4D26, 0x33},// -
+ {0x4E0E, 0x59},// -
+ {0x4E14, 0x55},// -
+ {0x4E16, 0x59},// -
+ {0x4E1E, 0x3B},// -
+ {0x4E20, 0x47},// -
+ {0x4E22, 0x54},// -
+ {0x4E26, 0x81},// -
+ {0x4E2C, 0x7D},// -
+ {0x4E2E, 0x81},// -
+ {0x4E36, 0x63},// -
+ {0x4E38, 0x6F},// -
+ {0x4E3A, 0x7C},// -
+ {0x4F3A, 0x3C},// -
+ {0x4F3C, 0x46},// -
+ {0x4F3E, 0x59},// -
+ {0x4F42, 0x64},// -
+ {0x4F44, 0x6E},// -
+ {0x4F46, 0x81},// -
+ {0x4F4A, 0x82},// -
+ {0x4F5A, 0x81},// -
+ {0x4F62, 0xAA},// -
+ {0x4F72, 0xA9},// -
+ {0x4F78, 0x36},// -
+ {0x4F7A, 0x41},// -
+ {0x4F7C, 0x61},// -
+ {0x4F7D, 0x01},// -
+ {0x4F7E, 0x7C},// -
+ {0x4F7F, 0x01},// -
+ {0x4F80, 0x77},// -
+ {0x4F82, 0x7B},// -
+ {0x4F88, 0x37},// -
+ {0x4F8A, 0x40},// -
+ {0x4F8C, 0x62},// -
+ {0x4F8D, 0x01},// -
+ {0x4F8E, 0x76},// -
+ {0x4F8F, 0x01},// -
+ {0x4F90, 0x5E},// -
+ {0x4F91, 0x02},// -
+ {0x4F92, 0x69},// -
+ {0x4F93, 0x02},// -
+ {0x4F94, 0x89},// -
+ {0x4F95, 0x02},// -
+ {0x4F96, 0xA4},// -
+ {0x4F97, 0x02},// -
+ {0x4F98, 0x9F},// -
+ {0x4F99, 0x02},// -
+ {0x4F9A, 0xA3},// -
+ {0x4F9B, 0x02},// -
+ {0x4FA0, 0x5F},// -
+ {0x4FA1, 0x02},// -
+ {0x4FA2, 0x68},// -
+ {0x4FA3, 0x02},// -
+ {0x4FA4, 0x8A},// -
+ {0x4FA5, 0x02},// -
+ {0x4FA6, 0x9E},// -
+ {0x4FA7, 0x02},// -
+ {0x519E, 0x79},// -
+ {0x51A6, 0xA1},// -
+ {0x51F0, 0xAC},// -
+ {0x51F2, 0xAA},// -
+ {0x51F4, 0xA5},// -
+ {0x51F6, 0xA0},// -
+ {0x5200, 0x9B},// -
+ {0x5202, 0x91},// -
+ {0x5204, 0x87},// -
+ {0x5206, 0x82},// -
+ {0x5208, 0xAC},// -
+ {0x520A, 0xAA},// -
+ {0x520C, 0xA5},// -
+ {0x520E, 0xA0},// -
+ {0x5210, 0x9B},// -
+ {0x5212, 0x91},// -
+ {0x5214, 0x87},// -
+ {0x5216, 0x82},// -
+ {0x5218, 0xAC},// -
+ {0x521A, 0xAA},// -
+ {0x521C, 0xA5},// -
+ {0x521E, 0xA0},// -
+ {0x5220, 0x9B},// -
+ {0x5222, 0x91},// -
+ {0x5224, 0x87},// -
+ {0x5226, 0x82},// -
+};
+
+/* Common Registers for ClearHDR. */
+static const struct imx585_reg common_clearHDR_mode[] = {
+ {0x301A, 0x10}, // WDMODE: Clear HDR mode
+ {0x3024, 0x02}, // COMBI_EN: 0x02
+ {0x3069, 0x02}, // Clear HDR mode
+ {0x3074, 0x63}, // Clear HDR mode
+ {0x3930, 0xE6}, // DUR[15:8]: Clear HDR mode (12bit)
+ {0x3931, 0x00}, // DUR[7:0]: Clear HDR mode (12bit)
+ {0x3A4C, 0x61}, // WAIT_ST0[7:0]: Clear HDR mode
+ {0x3A4D, 0x02}, // WAIT_ST0[15:8]: Clear HDR mode
+ {0x3A50, 0x70}, // WAIT_ST1[7:0]: Clear HDR mode
+ {0x3A51, 0x02}, // WAIT_ST1[15:8]: Clear HDR mode
+ {0x3E10, 0x17}, // ADTHEN: Clear HDR mode
+ {0x493C, 0x41}, // WAIT_10_SHF AD 10-bit 0x0C disable
+ {0x4940, 0x41}, // WAIT_12_SHF AD 12-bit 0x41 enable
+ {0x3081, 0x02}, // EXP_GAIN: High gain setting +12dB default
+};
+
+/* Common Registers for non-ClearHDR. */
+static const struct imx585_reg common_normal_mode[] = {
+ {0x301A, 0x00}, // WDMODE: Normal mode
+ {0x3024, 0x00}, // COMBI_EN: 0x00
+ {0x3069, 0x00}, // Normal mode
+ {0x3074, 0x64}, // Normal mode
+ {0x3930, 0x0C}, // DUR[15:8]: Normal mode (12bit)
+ {0x3931, 0x01}, // DUR[7:0]: Normal mode (12bit)
+ {0x3A4C, 0x39}, // WAIT_ST0[7:0]: Normal mode
+ {0x3A4D, 0x01}, // WAIT_ST0[15:8]: Normal mode
+ {0x3A50, 0x48}, // WAIT_ST1[7:0]: Normal mode
+ {0x3A51, 0x01}, // WAIT_ST1[15:8]: Normal mode
+ {0x3E10, 0x10}, // ADTHEN: Normal mode
+ {0x493C, 0x23}, // WAIT_10_SHF AD 10-bit 0x23 Normal mode
+ {0x4940, 0x23}, // WAIT_12_SHF AD 12-bit 0x23 Normal mode
+};
+
+/* All pixel 4K60. 12-bit */
+static const struct imx585_reg mode_4k_regs_12bit[] = {
+ {0x301B, 0x00}, // ADDMODE non-binning
+ {0x3022, 0x02}, // ADBIT 12-bit
+ {0x3023, 0x01}, // MDBIT 12-bit
+ {0x30D5, 0x04}, // DIG_CLP_VSTART non-binning
+};
+
+/* 2x2 binned 1080p60. 12-bit */
+static const struct imx585_reg mode_1080_regs_12bit[] = {
+ {0x301B, 0x01}, // ADDMODE binning
+ {0x3022, 0x02}, // ADBIT 12-bit
+ {0x3023, 0x01}, // MDBIT 12-bit
+ {0x30D5, 0x02}, // DIG_CLP_VSTART binning
+};
+
+/* IMX585 Register List - END*/
+
+/* For Mode List:
+ * Default:
+ * 12Bit - FHD, 4K
+ * ClearHDR Enabled:
+ * 12bit + Gradation compression
+ * 16bit - FHD, 4K
+ *
+ * Gradation compression is available on 12bit
+ * With Default option, only 12bit mode is exposed
+ * With ClearHDR enabled via parameters,
+ * 12bit will be with Gradation compression enabled
+ * 16bit mode exposed
+ *
+ * Technically, because the sensor is actually binning
+ * in digital domain, its readout speed is the same
+ * between 4K and FHD. However, through testing it is
+ * possible to "overclock" the FHD mode, thus leaving the
+ * hmax_div option for those who want to try.
+ * Also, note that FHD and 4K mode shared the same VMAX.
+ */
+
+/* Mode configs */
+struct imx585_mode supported_modes[] = {
+ {
+ /* 1080p60 2x2 binning */
+ .width = 1928,
+ .height = 1090,
+ .hmax_div = 1,
+ .min_HMAX = 366,
+ .min_VMAX = IMX585_VMAX_DEFAULT,
+ .default_HMAX = 366,
+ .default_VMAX = IMX585_VMAX_DEFAULT,
+ .crop = {
+ .left = IMX585_PIXEL_ARRAY_LEFT,
+ .top = IMX585_PIXEL_ARRAY_TOP,
+ .width = IMX585_PIXEL_ARRAY_WIDTH,
+ .height = IMX585_PIXEL_ARRAY_HEIGHT,
+ },
+ .reg_list = {
+ .num_of_regs = ARRAY_SIZE(mode_1080_regs_12bit),
+ .regs = mode_1080_regs_12bit,
+ },
+ },
+ {
+ /* 4K60 All pixel */
+ .width = 3856,
+ .height = 2180,
+ .min_HMAX = 550,
+ .min_VMAX = IMX585_VMAX_DEFAULT,
+ .default_HMAX = 550,
+ .default_VMAX = IMX585_VMAX_DEFAULT,
+ .hmax_div = 1,
+ .crop = {
+ .left = IMX585_PIXEL_ARRAY_LEFT,
+ .top = IMX585_PIXEL_ARRAY_TOP,
+ .width = IMX585_PIXEL_ARRAY_WIDTH,
+ .height = IMX585_PIXEL_ARRAY_HEIGHT,
+ },
+ .reg_list = {
+ .num_of_regs = ARRAY_SIZE(mode_4k_regs_12bit),
+ .regs = mode_4k_regs_12bit,
+ },
+ },
+};
+
+/*
+ * The supported formats.
+ * This table MUST contain 4 entries per format, to cover the various flip
+ * combinations in the order
+ * - no flip
+ * - h flip
+ * - v flip
+ * - h&v flips
+ */
+
+/* 12bit Only */
+static const u32 codes_normal[] = {
+ MEDIA_BUS_FMT_SRGGB12_1X12,
+ MEDIA_BUS_FMT_SGRBG12_1X12,
+ MEDIA_BUS_FMT_SGBRG12_1X12,
+ MEDIA_BUS_FMT_SBGGR12_1X12,
+};
+
+/* 12bit + 16bit for Clear HDR */
+static const u32 codes_clearhdr[] = {
+ /* 16-bit modes. */
+ MEDIA_BUS_FMT_SRGGB16_1X16,
+ MEDIA_BUS_FMT_SGRBG16_1X16,
+ MEDIA_BUS_FMT_SGBRG16_1X16,
+ MEDIA_BUS_FMT_SBGGR16_1X16,
+ /* 12-bit modes. */
+ MEDIA_BUS_FMT_SRGGB12_1X12,
+ MEDIA_BUS_FMT_SGRBG12_1X12,
+ MEDIA_BUS_FMT_SGBRG12_1X12,
+ MEDIA_BUS_FMT_SBGGR12_1X12,
+};
+
+/* Flip isn’t relevant for mono */
+static const u32 mono_codes[] = {
+ MEDIA_BUS_FMT_Y16_1X16, /* 16-bit mono */
+ MEDIA_BUS_FMT_Y12_1X12, /* 12-bit mono */
+};
+
+/* regulator supplies */
+static const char * const imx585_supply_name[] = {
+ /* Supplies can be enabled in any order */
+ "VANA", /* Analog (3.3V) supply */
+ "VDIG", /* Digital Core (1.1V) supply */
+ "VDDL", /* IF (1.8V) supply */
+};
+
+#define imx585_NUM_SUPPLIES ARRAY_SIZE(imx585_supply_name)
+
+struct imx585 {
+ struct v4l2_subdev sd;
+ struct media_pad pad[NUM_PADS];
+
+ unsigned int fmt_code;
+
+ struct clk *xclk;
+ u32 xclk_freq;
+
+ /* chosen INCK_SEL register value */
+ u8 inck_sel_val;
+
+ /* Link configurations */
+ unsigned int lane_count;
+ unsigned int link_freq_idx;
+
+ struct gpio_desc *reset_gpio;
+ struct regulator_bulk_data supplies[imx585_NUM_SUPPLIES];
+
+ struct v4l2_ctrl_handler ctrl_handler;
+
+ /* V4L2 Controls */
+ struct v4l2_ctrl *pixel_rate;
+ struct v4l2_ctrl *link_freq;
+ struct v4l2_ctrl *exposure;
+ struct v4l2_ctrl *gain;
+ struct v4l2_ctrl *hcg_ctrl;
+ struct v4l2_ctrl *vflip;
+ struct v4l2_ctrl *hflip;
+ struct v4l2_ctrl *vblank;
+ struct v4l2_ctrl *hblank;
+ struct v4l2_ctrl *blacklevel;
+
+ /* V4L2 HDR Controls */
+ struct v4l2_ctrl *hdr_mode;
+ struct v4l2_ctrl *datasel_th_ctrl;
+ struct v4l2_ctrl *datasel_bk_ctrl;
+ struct v4l2_ctrl *gdc_th_ctrl;
+ struct v4l2_ctrl *gdc_exp_ctrl_l;
+ struct v4l2_ctrl *gdc_exp_ctrl_h;
+ struct v4l2_ctrl *hdr_gain_ctrl;
+
+ /* V4L2 IR Cut filter switch Controls */
+ bool has_ircut;
+ struct v4l2_ctrl *ircut_ctrl;
+ struct i2c_client *ircut_client;
+
+ /* Current mode */
+ const struct imx585_mode *mode;
+
+ /* HCG enabled flag*/
+ bool hcg;
+
+ /* Mono mode */
+ bool mono;
+
+ /* Clear HDR mode */
+ bool clear_HDR;
+
+ /* Sync Mode*/
+ /* 0 = Internal Sync Leader Mode
+ * 1 = External Sync Leader Mode
+ * 2 = Follower Mode
+ * The datasheet wording is very confusing but basically:
+ * Leader Mode = Sensor using internal clock to drive the sensor
+ * But with external sync mode you can send a XVS input so the sensor
+ * will try to align with it.
+ * For Follower mode it is purely driven by external clock.
+ * In this case you need to drive both XVS and XHS.
+ */
+ u32 sync_mode;
+
+ /* Tracking sensor VMAX/HMAX value */
+ u16 HMAX;
+ u32 VMAX;
+
+ /*
+ * Mutex for serialized access:
+ * Protect sensor module set pad format and start/stop streaming safely.
+ */
+ struct mutex mutex;
+
+ /* Streaming on/off */
+ bool streaming;
+
+ /* Rewrite common registers on stream on? */
+ bool common_regs_written;
+};
+
+static inline struct imx585 *to_imx585(struct v4l2_subdev *_sd)
+{
+ return container_of(_sd, struct imx585, sd);
+}
+
+static inline void get_mode_table(struct imx585 *imx585, unsigned int code,
+ const struct imx585_mode **mode_list,
+ unsigned int *num_modes)
+{
+ *mode_list = NULL;
+ *num_modes = 0;
+
+ if (imx585->mono) {
+ /* --- Mono paths --- */
+ if (code == MEDIA_BUS_FMT_Y16_1X16 && imx585->clear_HDR) {
+ *mode_list = supported_modes;
+ *num_modes = ARRAY_SIZE(supported_modes);
+ }
+ if (code == MEDIA_BUS_FMT_Y12_1X12) {
+ *mode_list = supported_modes;
+ *num_modes = ARRAY_SIZE(supported_modes);
+ }
+ } else {
+ /* --- Color paths --- */
+ switch (code) {
+ /* 16-bit */
+ case MEDIA_BUS_FMT_SRGGB16_1X16:
+ case MEDIA_BUS_FMT_SGRBG16_1X16:
+ case MEDIA_BUS_FMT_SGBRG16_1X16:
+ case MEDIA_BUS_FMT_SBGGR16_1X16:
+ /* 12-bit */
+ case MEDIA_BUS_FMT_SRGGB12_1X12:
+ case MEDIA_BUS_FMT_SGRBG12_1X12:
+ case MEDIA_BUS_FMT_SGBRG12_1X12:
+ case MEDIA_BUS_FMT_SBGGR12_1X12:
+ *mode_list = supported_modes;
+ *num_modes = ARRAY_SIZE(supported_modes);
+ break;
+ default:
+ *mode_list = NULL;
+ *num_modes = 0;
+ }
+ }
+}
+
+/* ------------------------------------------------------------------
+ * Optional IR-cut helper
+ * ------------------------------------------------------------------
+ */
+
+/* One-byte “command” sent to the IR-cut MCU at imx585->ircut_client */
+static int imx585_ircut_write(struct imx585 *imx585, u8 cmd)
+{
+ struct i2c_client *client = imx585->ircut_client;
+ int ret;
+
+ ret = i2c_smbus_write_byte(client, cmd);
+ if (ret < 0)
+ dev_err(&client->dev, "IR-cut write failed (%d)\n", ret);
+
+ return ret;
+}
+
+static int imx585_ircut_set(struct imx585 *imx585, int on)
+{
+ return imx585_ircut_write(imx585, on ? 0x01 : 0x00);
+}
+
+/* Read registers up to 2 at a time */
+static int imx585_read_reg(struct imx585 *imx585, u16 reg, u32 len, u32 *val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ struct i2c_msg msgs[2];
+ u8 addr_buf[2] = { reg >> 8, reg & 0xff };
+ u8 data_buf[4] = { 0, };
+ int ret;
+
+ if (len > 4)
+ return -EINVAL;
+
+ /* Write register address */
+ msgs[0].addr = client->addr;
+ msgs[0].flags = 0;
+ msgs[0].len = ARRAY_SIZE(addr_buf);
+ msgs[0].buf = addr_buf;
+
+ /* Read data from register */
+ msgs[1].addr = client->addr;
+ msgs[1].flags = I2C_M_RD;
+ msgs[1].len = len;
+ msgs[1].buf = &data_buf[4 - len];
+
+ ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+ if (ret != ARRAY_SIZE(msgs))
+ return -EIO;
+
+ *val = get_unaligned_be32(data_buf);
+
+ return 0;
+}
+
+/* Write registers 1 byte at a time */
+static int imx585_write_reg_1byte(struct imx585 *imx585, u16 reg, u8 val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ u8 buf[3];
+ int ret;
+
+ put_unaligned_be16(reg, buf);
+ buf[2] = val;
+ ret = i2c_master_send(client, buf, 3);
+ if (ret != 3)
+ return ret;
+
+ return 0;
+}
+
+/* Write registers 2 byte at a time */
+static int imx585_write_reg_2byte(struct imx585 *imx585, u16 reg, u16 val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ u8 buf[4];
+ int ret;
+
+ put_unaligned_be16(reg, buf);
+ buf[2] = val;
+ buf[3] = val >> 8;
+ ret = i2c_master_send(client, buf, 4);
+ if (ret != 4)
+ return ret;
+
+ return 0;
+}
+
+/* Write registers 3 byte at a time */
+static int imx585_write_reg_3byte(struct imx585 *imx585, u16 reg, u32 val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ u8 buf[5];
+
+ put_unaligned_be16(reg, buf);
+ buf[2] = val;
+ buf[3] = val >> 8;
+ buf[4] = val >> 16;
+ if (i2c_master_send(client, buf, 5) != 5)
+ return -EIO;
+
+ return 0;
+}
+
+/* Write a list of 1 byte registers */
+static int imx585_write_regs(struct imx585 *imx585,
+ const struct imx585_reg *regs, u32 len)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < len; i++) {
+ ret = imx585_write_reg_1byte(imx585, regs[i].address,
+ regs[i].val);
+ if (ret) {
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ regs[i].address, ret);
+
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+/* Hold register values until hold is disabled */
+static inline void imx585_register_hold(struct imx585 *imx585, bool hold)
+{
+ imx585_write_reg_1byte(imx585, 0x3001, hold ? 1 : 0);
+}
+
+/* Get bayer order based on flip setting. */
+static u32 imx585_get_format_code(struct imx585 *imx585, u32 code)
+{
+ unsigned int i;
+
+ lockdep_assert_held(&imx585->mutex);
+
+ if (imx585->mono) {
+ for (i = 0; i < ARRAY_SIZE(mono_codes); i++)
+ if (mono_codes[i] == code)
+ break;
+ return mono_codes[i];
+ }
+
+ if (imx585->clear_HDR) {
+ for (i = 0; i < ARRAY_SIZE(codes_clearhdr); i++)
+ if (codes_clearhdr[i] == code)
+ break;
+ return codes_clearhdr[i];
+ }
+
+ for (i = 0; i < ARRAY_SIZE(codes_normal); i++)
+ if (codes_normal[i] == code)
+ break;
+ return codes_normal[i];
+}
+
+static void imx585_set_default_format(struct imx585 *imx585)
+{
+ /* Set default mode to max resolution */
+ imx585->mode = &supported_modes[0];
+ if (imx585->mono)
+ imx585->fmt_code = MEDIA_BUS_FMT_Y12_1X12;
+ else
+ imx585->fmt_code = MEDIA_BUS_FMT_SRGGB12_1X12;
+}
+
+static int imx585_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+ struct imx585 *imx585 = to_imx585(sd);
+ struct v4l2_mbus_framefmt *try_fmt_img =
+ v4l2_subdev_state_get_format(fh->state, IMAGE_PAD);
+ struct v4l2_mbus_framefmt *try_fmt_meta =
+ v4l2_subdev_state_get_format(fh->state, METADATA_PAD);
+ struct v4l2_rect *try_crop;
+
+ mutex_lock(&imx585->mutex);
+
+ /* Initialize try_fmt for the image pad */
+ try_fmt_img->width = supported_modes[0].width;
+ try_fmt_img->height = supported_modes[0].height;
+ if (imx585->mono)
+ try_fmt_img->code = imx585_get_format_code(imx585, MEDIA_BUS_FMT_Y12_1X12);
+ else
+ try_fmt_img->code = imx585_get_format_code(imx585, MEDIA_BUS_FMT_SRGGB12_1X12);
+
+ try_fmt_img->field = V4L2_FIELD_NONE;
+
+ /* Initialize try_fmt for the embedded metadata pad */
+ try_fmt_meta->width = IMX585_EMBEDDED_LINE_WIDTH;
+ try_fmt_meta->height = IMX585_NUM_EMBEDDED_LINES;
+ try_fmt_meta->code = MEDIA_BUS_FMT_SENSOR_DATA;
+ try_fmt_meta->field = V4L2_FIELD_NONE;
+
+ /* Initialize try_crop */
+ try_crop = v4l2_subdev_state_get_crop(fh->state, IMAGE_PAD);
+ try_crop->left = IMX585_PIXEL_ARRAY_LEFT;
+ try_crop->top = IMX585_PIXEL_ARRAY_TOP;
+ try_crop->width = IMX585_PIXEL_ARRAY_WIDTH;
+ try_crop->height = IMX585_PIXEL_ARRAY_HEIGHT;
+
+ mutex_unlock(&imx585->mutex);
+
+ return 0;
+}
+
+/* For HDR mode, Gain is limited to 0~80 and HCG is disabled
+ * For Normal mode, Gain is limited to 0~240
+ */
+static void imx585_update_gain_limits(struct imx585 *imx585)
+{
+ bool hcg_on = imx585->hcg;
+ bool clear_hdr = imx585->clear_HDR;
+ u32 min = hcg_on ? IMX585_ANA_GAIN_MIN_HCG : IMX585_ANA_GAIN_MIN_NORMAL;
+ u32 max = clear_hdr ? IMX585_ANA_GAIN_MAX_HDR : IMX585_ANA_GAIN_MAX_NORMAL;
+ u32 cur = imx585->gain->val;
+
+ __v4l2_ctrl_modify_range(imx585->gain,
+ min, max,
+ IMX585_ANA_GAIN_STEP,
+ clamp(cur, min, max));
+
+ if (cur < min || cur > max)
+ __v4l2_ctrl_s_ctrl(imx585->gain,
+ clamp(cur, min, max));
+}
+
+static void imx585_update_hmax(struct imx585 *imx585)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+
+ const u32 base_4lane = HMAX_table_4lane_4K[imx585->link_freq_idx];
+ const u32 lane_scale = (imx585->lane_count == 2) ? 2 : 1;
+ const u32 factor = base_4lane * lane_scale;
+ const u32 hdr_factor = (imx585->clear_HDR) ? 2 : 1;
+
+ dev_info(&client->dev, "Upadte minimum HMAX\n");
+ dev_info(&client->dev, "\tbase_4lane: %d\n", base_4lane);
+ dev_info(&client->dev, "\tlane_scale: %d\n", lane_scale);
+ dev_info(&client->dev, "\tfactor: %d\n", factor);
+ dev_info(&client->dev, "\thdr_factor: %d\n", hdr_factor);
+
+ for (unsigned int i = 0; i < ARRAY_SIZE(supported_modes); ++i) {
+ u32 h = factor / supported_modes[i].hmax_div;
+ u64 v = IMX585_VMAX_DEFAULT * hdr_factor;
+
+ supported_modes[i].min_HMAX = h;
+ supported_modes[i].default_HMAX = h;
+ supported_modes[i].min_VMAX = v;
+ supported_modes[i].default_VMAX = v;
+ dev_info(&client->dev, "\tvmax: %lld x hmax: %d\n", v, h);
+ }
+}
+
+static void imx585_set_framing_limits(struct imx585 *imx585)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ const struct imx585_mode *mode = imx585->mode;
+ u64 default_hblank, max_hblank;
+ u64 pixel_rate;
+
+ imx585_update_hmax(imx585);
+
+ dev_info(&client->dev, "mode: %d x %d\n", mode->width, mode->height);
+
+ imx585->VMAX = mode->default_VMAX;
+ imx585->HMAX = mode->default_HMAX;
+
+ pixel_rate = (u64)mode->width * IMX585_PIXEL_RATE;
+ /* In the case where ClearHDR is enabled, HMAX is effectly doubled */
+ /* So pixel rate is half with the same HMAX with ClearHDR */
+ do_div(pixel_rate, mode->min_HMAX);
+ __v4l2_ctrl_modify_range(imx585->pixel_rate, pixel_rate, pixel_rate, 1, pixel_rate);
+
+ //int default_hblank = mode->default_HMAX*IMX585_PIXEL_RATE/72000000-IMX585_NATIVE_WIDTH;
+ default_hblank = mode->default_HMAX * pixel_rate;
+ do_div(default_hblank, IMX585_PIXEL_RATE);
+ default_hblank = default_hblank - mode->width;
+
+ max_hblank = IMX585_HMAX_MAX * pixel_rate;
+ do_div(max_hblank, IMX585_PIXEL_RATE);
+ max_hblank = max_hblank - mode->width;
+
+ __v4l2_ctrl_modify_range(imx585->hblank, 0, max_hblank, 1, default_hblank);
+ __v4l2_ctrl_s_ctrl(imx585->hblank, default_hblank);
+
+ /* Update limits and set FPS to default */
+ __v4l2_ctrl_modify_range(imx585->vblank, mode->min_VMAX - mode->height,
+ IMX585_VMAX_MAX - mode->height,
+ 1, mode->default_VMAX - mode->height);
+ __v4l2_ctrl_s_ctrl(imx585->vblank, mode->default_VMAX - mode->height);
+
+ __v4l2_ctrl_modify_range(imx585->exposure, IMX585_EXPOSURE_MIN,
+ imx585->VMAX - IMX585_SHR_MIN_HDR, 1,
+ IMX585_EXPOSURE_DEFAULT);
+ dev_info(&client->dev, "default vmax: %lld x hmax: %d\n", mode->min_VMAX, mode->min_HMAX);
+ dev_info(&client->dev, "Setting default HBLANK : %llu, VBLANK : %llu PixelRate: %lld\n",
+ default_hblank, mode->default_VMAX - mode->height, pixel_rate);
+}
+
+static int imx585_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct imx585 *imx585 = container_of(ctrl->handler, struct imx585, ctrl_handler);
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ const struct imx585_mode *mode = imx585->mode;
+ const struct imx585_mode *mode_list;
+ unsigned int code, num_modes;
+
+ int ret = 0;
+ /*
+ * Applying V4L2 control value that
+ * doesn't need to be in streaming mode
+ */
+ switch (ctrl->id) {
+ case V4L2_CID_WIDE_DYNAMIC_RANGE:
+ if (imx585->clear_HDR != ctrl->val) {
+ imx585->clear_HDR = ctrl->val;
+ v4l2_ctrl_activate(imx585->datasel_th_ctrl, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->datasel_bk_ctrl, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->gdc_th_ctrl, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->gdc_exp_ctrl_h, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->gdc_exp_ctrl_l, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->hdr_gain_ctrl, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->hcg_ctrl, !imx585->clear_HDR);
+ imx585_update_gain_limits(imx585);
+ if (imx585->mono)
+ code = imx585_get_format_code(imx585, MEDIA_BUS_FMT_Y12_1X12);
+ else
+ code = imx585_get_format_code(imx585, MEDIA_BUS_FMT_SRGGB12_1X12);
+ get_mode_table(imx585, code, &mode_list, &num_modes);
+ imx585->mode = v4l2_find_nearest_size(mode_list,
+ num_modes,
+ width, height,
+ imx585->mode->width,
+ imx585->mode->height);
+ imx585_set_framing_limits(imx585);
+ }
+ break;
+ }
+
+ /*
+ * Applying V4L2 control value only happens
+ * when power is up for streaming
+ */
+ if (pm_runtime_get_if_in_use(&client->dev) == 0)
+ return 0;
+
+ switch (ctrl->id) {
+ case V4L2_CID_EXPOSURE:
+ {
+ u32 shr;
+
+ shr = (imx585->VMAX - ctrl->val) & ~1u; //Always a multiple of 2
+ dev_info(&client->dev, "V4L2_CID_EXPOSURE : %d\n", ctrl->val);
+ dev_info(&client->dev, "\tVMAX:%d, HMAX:%d\n", imx585->VMAX, imx585->HMAX);
+ dev_info(&client->dev, "\tSHR:%d\n", shr);
+
+ ret = imx585_write_reg_3byte(imx585, IMX585_REG_SHR, shr);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_SHR, ret);
+ break;
+ }
+ case V4L2_CID_IMX585_HCG_GAIN:
+ {
+ if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE)
+ break;
+ imx585->hcg = ctrl->val;
+ imx585_update_gain_limits(imx585);
+
+ // Set HCG/LCG channel
+ ret = imx585_write_reg_1byte(imx585, IMX585_REG_FDG_SEL0, ctrl->val);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_FDG_SEL0, ret);
+ dev_info(&client->dev, "V4L2_CID_HCG_ENABLE: %d\n", ctrl->val);
+ break;
+ }
+ case V4L2_CID_ANALOGUE_GAIN:
+ {
+ u32 gain = ctrl->val;
+
+ dev_info(&client->dev, "analogue gain = %u (%s)\n",
+ gain, imx585->hcg ? "HCG" : "LCG");
+
+ ret = imx585_write_reg_2byte(imx585, IMX585_REG_ANALOG_GAIN, gain);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "ANALOG_GAIN write failed (%d)\n", ret);
+ break;
+ }
+ case V4L2_CID_VBLANK:
+ {
+ u32 current_exposure = imx585->exposure->cur.val;
+ u32 min_SHR = (imx585->clear_HDR) ? IMX585_SHR_MIN_HDR : IMX585_SHR_MIN;
+ /*
+ * The VBLANK control may change the limits of usable exposure, so check
+ * and adjust if necessary.
+ */
+ imx585->VMAX = (mode->height + ctrl->val) & ~1u; //Always a multiple of 2
+
+ /* New maximum exposure limits,
+ * modifying the range and make sure we are not exceed the new maximum.
+ */
+ current_exposure = clamp_t(u32, current_exposure, IMX585_EXPOSURE_MIN,
+ imx585->VMAX - min_SHR);
+ __v4l2_ctrl_modify_range(imx585->exposure, IMX585_EXPOSURE_MIN,
+ imx585->VMAX - min_SHR, 1,
+ current_exposure);
+
+ dev_info(&client->dev, "V4L2_CID_VBLANK : %d\n", ctrl->val);
+ dev_info(&client->dev, "\tVMAX:%d, HMAX:%d\n", imx585->VMAX, imx585->HMAX);
+ dev_info(&client->dev, "Update exposure limits: max:%d, min:%d, current:%d\n",
+ imx585->VMAX - min_SHR,
+ IMX585_EXPOSURE_MIN, current_exposure);
+
+ ret = imx585_write_reg_3byte(imx585, IMX585_REG_VMAX, imx585->VMAX);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_VMAX, ret);
+ break;
+ }
+ case V4L2_CID_HBLANK:
+ {
+ u64 pixel_rate;
+ u64 hmax;
+
+ pixel_rate = (u64)mode->width * IMX585_PIXEL_RATE;
+ do_div(pixel_rate, mode->min_HMAX);
+ hmax = (u64)(mode->width + ctrl->val) * IMX585_PIXEL_RATE;
+ do_div(hmax, pixel_rate);
+ imx585->HMAX = hmax;
+
+ dev_info(&client->dev, "V4L2_CID_HBLANK : %d\n", ctrl->val);
+ dev_info(&client->dev, "\tHMAX : %d\n", imx585->HMAX);
+
+ ret = imx585_write_reg_2byte(imx585, IMX585_REG_HMAX, hmax);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_HMAX, ret);
+ break;
+ }
+ case V4L2_CID_HFLIP:
+ dev_info(&client->dev, "V4L2_CID_HFLIP : %d\n", ctrl->val);
+ ret = imx585_write_reg_1byte(imx585, IMX585_FLIP_WINMODEH, ctrl->val);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_FLIP_WINMODEH, ret);
+ break;
+ case V4L2_CID_VFLIP:
+ dev_info(&client->dev, "V4L2_CID_VFLIP : %d\n", ctrl->val);
+ ret = imx585_write_reg_1byte(imx585, IMX585_FLIP_WINMODEV, ctrl->val);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_FLIP_WINMODEV, ret);
+ break;
+ case V4L2_CID_BRIGHTNESS:
+ {
+ u16 blacklevel = ctrl->val;
+
+ dev_info(&client->dev, "V4L2_CID_BRIGHTNESS : %d\n", ctrl->val);
+
+ if (blacklevel > 4095)
+ blacklevel = 4095;
+ ret = imx585_write_reg_1byte(imx585, IMX585_REG_BLKLEVEL, blacklevel);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_BLKLEVEL, ret);
+ break;
+ }
+ case V4L2_CID_BAND_STOP_FILTER:
+ if (imx585->has_ircut) {
+ dev_info(&client->dev, "V4L2_CID_BAND_STOP_FILTER : %d\n", ctrl->val);
+ imx585_ircut_set(imx585, ctrl->val);
+ }
+ break;
+ case V4L2_CID_IMX585_HDR_DATASEL_TH:{
+ const u16 *th = (const u16 *)ctrl->p_new.p;
+
+ ret = imx585_write_reg_2byte(imx585, IMX585_REG_EXP_TH_H, th[0]);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_EXP_TH_H, ret);
+ ret = imx585_write_reg_2byte(imx585, IMX585_REG_EXP_TH_L, th[1]);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_EXP_TH_L, ret);
+ dev_info(&client->dev, "V4L2_CID_IMX585_HDR_DATASEL_TH : %d, %d\n", th[0], th[1]);
+ break;
+ }
+ case V4L2_CID_IMX585_HDR_DATASEL_BK:
+ ret = imx585_write_reg_1byte(imx585, IMX585_REG_EXP_BK, ctrl->val);
+ dev_info(&client->dev, "V4L2_CID_IMX585_HDR_DATASEL_BK : %d\n", ctrl->val);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_EXP_BK, ret);
+ break;
+ case V4L2_CID_IMX585_HDR_GRAD_TH:{
+ const u32 *thr = (const u32 *)ctrl->p_new.p;
+
+ ret = imx585_write_reg_3byte(imx585, IMX585_REG_CCMP1_EXP, thr[0]);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_CCMP1_EXP, ret);
+ ret = imx585_write_reg_3byte(imx585, IMX585_REG_CCMP2_EXP, thr[1]);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_CCMP2_EXP, ret);
+ dev_info(&client->dev, "V4L2_CID_IMX585_HDR_GRAD_TH : %d, %d\n", thr[0], thr[1]);
+ break;
+ }
+ case V4L2_CID_IMX585_HDR_GRAD_COMP_L:{
+ ret = imx585_write_reg_1byte(imx585, IMX585_REG_ACMP1_EXP, ctrl->val);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_ACMP1_EXP, ret);
+ dev_info(&client->dev, "V4L2_CID_IMX585_HDR_GRAD_COMP_L : %d\n", ctrl->val);
+ break;
+ }
+ case V4L2_CID_IMX585_HDR_GRAD_COMP_H:{
+ ret = imx585_write_reg_1byte(imx585, IMX585_REG_ACMP2_EXP, ctrl->val);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_ACMP2_EXP, ret);
+ dev_info(&client->dev, "V4L2_CID_IMX585_HDR_GRAD_COMP_H : %d\n", ctrl->val);
+ break;
+ }
+ case V4L2_CID_IMX585_HDR_GAIN:
+ ret = imx585_write_reg_1byte(imx585, IMX585_REG_EXP_GAIN, ctrl->val);
+ dev_info(&client->dev, "IMX585_REG_EXP_GAIN : %d\n", ctrl->val);
+ if (ret)
+ dev_err_ratelimited(&client->dev,
+ "Failed to write reg 0x%4.4x. error = %d\n",
+ IMX585_REG_EXP_GAIN, ret);
+ break;
+ case V4L2_CID_WIDE_DYNAMIC_RANGE:
+ /* Already handled above. */
+ break;
+ default:
+ dev_info(&client->dev,
+ "ctrl(id:0x%x,val:0x%x) is not handled\n",
+ ctrl->id, ctrl->val);
+ break;
+ }
+
+ pm_runtime_put(&client->dev);
+
+ return ret;
+}
+
+static const struct v4l2_ctrl_ops imx585_ctrl_ops = {
+ .s_ctrl = imx585_set_ctrl,
+};
+
+static const u16 hdr_thresh_def[2] = { 512, 1024 };
+static const struct v4l2_ctrl_config imx585_cfg_datasel_th = {
+ .ops = &imx585_ctrl_ops,
+ .id = V4L2_CID_IMX585_HDR_DATASEL_TH,
+ .name = "HDR Data selection Threshold",
+ .type = V4L2_CTRL_TYPE_U16,
+ .min = 0,
+ .max = 0x0FFF,
+ .step = 1,
+ .def = 0,
+ .dims = { 2 },
+ .elem_size = sizeof(u16),
+};
+
+static const struct v4l2_ctrl_config imx585_cfg_datasel_bk = {
+ .ops = &imx585_ctrl_ops,
+ .id = V4L2_CID_IMX585_HDR_DATASEL_BK,
+ .name = "HDR Data Blending Mode",
+ .type = V4L2_CTRL_TYPE_MENU,
+ .max = ARRAY_SIZE(hdr_data_blender_menu) - 1,
+ .menu_skip_mask = 0,
+ .def = 0,
+ .qmenu = hdr_data_blender_menu,
+};
+
+static const u32 grad_thresh_def[2] = { 500, 11500 };
+static const struct v4l2_ctrl_config imx585_cfg_grad_th = {
+ .ops = &imx585_ctrl_ops,
+ .id = V4L2_CID_IMX585_HDR_GRAD_TH,
+ .name = "Gradiant Compression Threshold (16bit)",
+ .type = V4L2_CTRL_TYPE_U32,
+ .min = 0,
+ .max = 0x1FFFF,
+ .step = 1,
+ .def = 0,
+ .dims = { 2 },
+ .elem_size = sizeof(u32),
+};
+
+static const struct v4l2_ctrl_config imx585_cfg_grad_exp_l = {
+ .ops = &imx585_ctrl_ops,
+ .id = V4L2_CID_IMX585_HDR_GRAD_COMP_L,
+ .name = "Gradiant Compression Ratio Low",
+ .type = V4L2_CTRL_TYPE_MENU,
+ .min = 0,
+ .max = ARRAY_SIZE(grad_compression_slope_menu) - 1,
+ .menu_skip_mask = 0,
+ .def = 2,
+ .qmenu = grad_compression_slope_menu,
+};
+
+static const struct v4l2_ctrl_config imx585_cfg_grad_exp_h = {
+ .ops = &imx585_ctrl_ops,
+ .id = V4L2_CID_IMX585_HDR_GRAD_COMP_H,
+ .name = "Gradiant Compression Ratio High",
+ .type = V4L2_CTRL_TYPE_MENU,
+ .min = 0,
+ .max = ARRAY_SIZE(grad_compression_slope_menu) - 1,
+ .menu_skip_mask = 0,
+ .def = 6,
+ .qmenu = grad_compression_slope_menu,
+};
+
+static const struct v4l2_ctrl_config imx585_cfg_hdr_gain = {
+ .ops = &imx585_ctrl_ops,
+ .id = V4L2_CID_IMX585_HDR_GAIN,
+ .name = "HDR Gain Adder (dB)",
+ .type = V4L2_CTRL_TYPE_MENU,
+ .min = 0,
+ .max = ARRAY_SIZE(hdr_gain_adder_menu) - 1,
+ .menu_skip_mask = 0,
+ .def = 2,
+ .qmenu = hdr_gain_adder_menu,
+};
+
+static const struct v4l2_ctrl_config imx585_cfg_hcg = {
+ .ops = &imx585_ctrl_ops,
+ .id = V4L2_CID_IMX585_HCG_GAIN,
+ .name = "HCG Enable",
+ .type = V4L2_CTRL_TYPE_BOOLEAN,
+ .min = 0,
+ .max = 1,
+ .step = 1,
+ .def = 0,
+};
+
+static int imx585_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ struct imx585 *imx585 = to_imx585(sd);
+ unsigned int entries;
+ const u32 *tbl;
+
+ if (code->pad >= NUM_PADS)
+ return -EINVAL;
+
+ if (code->pad == IMAGE_PAD) {
+ if (imx585->mono) {
+ if (imx585->clear_HDR) {
+ if (code->index > 1)
+ return -EINVAL;
+ code->code = mono_codes[code->index];
+ return 0;
+ }
+ /* HDR off: expose Y12 only */
+ if (code->index)
+ return -EINVAL;
+
+ code->code = MEDIA_BUS_FMT_Y12_1X12;
+ return 0;
+ }
+
+ if (imx585->clear_HDR) {
+ tbl = codes_clearhdr; /* << 16bit + 12bit */
+ entries = ARRAY_SIZE(codes_clearhdr) / 4;
+ } else {
+ tbl = codes_normal; /* << ONLY 12bit */
+ entries = ARRAY_SIZE(codes_normal) / 4;
+ }
+
+ if (code->index >= entries)
+ return -EINVAL;
+
+ code->code = imx585_get_format_code(imx585, tbl[code->index * 4]);
+ return 0;
+ }
+ /* --- Metadata pad ------------------------------------------------- */
+ if (code->index)
+ return -EINVAL;
+
+ code->code = MEDIA_BUS_FMT_SENSOR_DATA;
+ return 0;
+}
+
+static int imx585_enum_frame_size(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_frame_size_enum *fse)
+{
+ struct imx585 *imx585 = to_imx585(sd);
+
+ if (fse->pad >= NUM_PADS)
+ return -EINVAL;
+
+ if (fse->pad == IMAGE_PAD) {
+ const struct imx585_mode *mode_list;
+ unsigned int num_modes;
+
+ get_mode_table(imx585, fse->code, &mode_list, &num_modes);
+
+ if (fse->index >= num_modes)
+ return -EINVAL;
+
+ if (fse->code != imx585_get_format_code(imx585, fse->code))
+ return -EINVAL;
+
+ fse->min_width = mode_list[fse->index].width;
+ fse->max_width = fse->min_width;
+ fse->min_height = mode_list[fse->index].height;
+ fse->max_height = fse->min_height;
+ } else {
+ if (fse->code != MEDIA_BUS_FMT_SENSOR_DATA || fse->index > 0)
+ return -EINVAL;
+
+ fse->min_width = IMX585_EMBEDDED_LINE_WIDTH;
+ fse->max_width = fse->min_width;
+ fse->min_height = IMX585_NUM_EMBEDDED_LINES;
+ fse->max_height = fse->min_height;
+ }
+
+ return 0;
+}
+
+static void imx585_reset_colorspace(const struct imx585_mode *mode, struct v4l2_mbus_framefmt *fmt)
+{
+ fmt->colorspace = V4L2_COLORSPACE_RAW;
+ fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
+ fmt->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
+ fmt->colorspace,
+ fmt->ycbcr_enc);
+ fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
+}
+
+static void imx585_update_image_pad_format(struct imx585 *imx585,
+ const struct imx585_mode *mode,
+ struct v4l2_subdev_format *fmt)
+{
+ fmt->format.width = mode->width;
+ fmt->format.height = mode->height;
+ fmt->format.field = V4L2_FIELD_NONE;
+ imx585_reset_colorspace(mode, &fmt->format);
+}
+
+static void imx585_update_metadata_pad_format(struct v4l2_subdev_format *fmt)
+{
+ fmt->format.width = IMX585_EMBEDDED_LINE_WIDTH;
+ fmt->format.height = IMX585_NUM_EMBEDDED_LINES;
+ fmt->format.code = MEDIA_BUS_FMT_SENSOR_DATA;
+ fmt->format.field = V4L2_FIELD_NONE;
+}
+
+static int imx585_get_pad_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_format *fmt)
+{
+ struct imx585 *imx585 = to_imx585(sd);
+
+ if (fmt->pad >= NUM_PADS)
+ return -EINVAL;
+
+ mutex_lock(&imx585->mutex);
+
+ if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+ struct v4l2_mbus_framefmt *try_fmt =
+ v4l2_subdev_state_get_format(sd_state, fmt->pad);
+ /* update the code which could change due to vflip or hflip: */
+ try_fmt->code = fmt->pad == IMAGE_PAD ?
+ imx585_get_format_code(imx585, try_fmt->code) :
+ MEDIA_BUS_FMT_SENSOR_DATA;
+ fmt->format = *try_fmt;
+ } else {
+ if (fmt->pad == IMAGE_PAD) {
+ imx585_update_image_pad_format(imx585, imx585->mode, fmt);
+ fmt->format.code =
+ imx585_get_format_code(imx585, imx585->fmt_code);
+ } else {
+ imx585_update_metadata_pad_format(fmt);
+ }
+ }
+
+ mutex_unlock(&imx585->mutex);
+ return 0;
+}
+
+static int imx585_set_pad_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_format *fmt)
+{
+ struct v4l2_mbus_framefmt *framefmt;
+ const struct imx585_mode *mode;
+ struct imx585 *imx585 = to_imx585(sd);
+
+ if (fmt->pad >= NUM_PADS)
+ return -EINVAL;
+
+ mutex_lock(&imx585->mutex);
+
+ if (fmt->pad == IMAGE_PAD) {
+ const struct imx585_mode *mode_list;
+ unsigned int num_modes;
+
+ /* Bayer order varies with flips */
+ fmt->format.code = imx585_get_format_code(imx585, fmt->format.code);
+ get_mode_table(imx585, fmt->format.code, &mode_list, &num_modes);
+ mode = v4l2_find_nearest_size(mode_list,
+ num_modes,
+ width, height,
+ fmt->format.width,
+ fmt->format.height);
+ imx585_update_image_pad_format(imx585, mode, fmt);
+ if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+ framefmt = v4l2_subdev_state_get_format(sd_state, fmt->pad);
+ *framefmt = fmt->format;
+ } else if (imx585->mode != mode ||
+ imx585->fmt_code != fmt->format.code) {
+ imx585->mode = mode;
+ imx585->fmt_code = fmt->format.code;
+ imx585_set_framing_limits(imx585);
+ }
+ } else {
+ if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+ framefmt = v4l2_subdev_state_get_format(sd_state, fmt->pad);
+ *framefmt = fmt->format;
+ } else {
+ /* Only one embedded data mode is supported */
+ imx585_update_metadata_pad_format(fmt);
+ }
+ }
+
+ mutex_unlock(&imx585->mutex);
+
+ return 0;
+}
+
+static const struct v4l2_rect *
+__imx585_get_pad_crop(struct imx585 *imx585,
+ struct v4l2_subdev_state *sd_state,
+ unsigned int pad, enum v4l2_subdev_format_whence which)
+{
+ switch (which) {
+ case V4L2_SUBDEV_FORMAT_TRY:
+ return v4l2_subdev_state_get_crop(sd_state, IMAGE_PAD);
+ case V4L2_SUBDEV_FORMAT_ACTIVE:
+ return &imx585->mode->crop;
+ }
+
+ return NULL;
+}
+
+/* Start streaming */
+static int imx585_start_streaming(struct imx585 *imx585)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ const struct IMX585_reg_list *reg_list;
+ int ret;
+
+ if (!imx585->common_regs_written) {
+ ret = imx585_write_regs(imx585, common_regs, ARRAY_SIZE(common_regs));
+ if (ret) {
+ dev_err(&client->dev, "%s failed to set common settings\n", __func__);
+ return ret;
+ }
+
+ imx585_write_reg_1byte(imx585, IMX585_INCK_SEL, imx585->inck_sel_val);
+ imx585_write_reg_2byte(imx585, IMX585_REG_BLKLEVEL, IMX585_BLKLEVEL_DEFAULT);
+ imx585_write_reg_1byte(imx585, IMX585_DATARATE_SEL,
+ link_freqs_reg_value[imx585->link_freq_idx]);
+
+ if (imx585->lane_count == 2)
+ imx585_write_reg_1byte(imx585, IMX585_LANEMODE, 0x01);
+ else
+ imx585_write_reg_1byte(imx585, IMX585_LANEMODE, 0x03);
+
+ if (imx585->mono)
+ imx585_write_reg_1byte(imx585, IMX585_BIN_MODE, 0x01);
+ else
+ imx585_write_reg_1byte(imx585, IMX585_BIN_MODE, 0x00);
+
+ if (imx585->sync_mode == 1) { //External Sync Leader Mode
+ dev_info(&client->dev, "External Sync Leader Mode, enable XVS input\n");
+ imx585_write_reg_1byte(imx585, IMX585_REG_EXTMODE, 0x01);
+ // Enable XHS output, but XVS is input
+ imx585_write_reg_1byte(imx585, IMX585_REG_XXS_DRV, 0x03);
+ // Disable XVS OUT
+ imx585_write_reg_1byte(imx585, IMX585_REG_XXS_OUTSEL, 0x08);
+ } else if (imx585->sync_mode == 0) { //Internal Sync Leader Mode
+ dev_info(&client->dev, "Internal Sync Leader Mode, enable output\n");
+ imx585_write_reg_1byte(imx585, IMX585_REG_EXTMODE, 0x00);
+ // Enable XHS and XVS output
+ imx585_write_reg_1byte(imx585, IMX585_REG_XXS_DRV, 0x00);
+ imx585_write_reg_1byte(imx585, IMX585_REG_XXS_OUTSEL, 0x0A);
+ } else {
+ dev_info(&client->dev, "Follower Mode, enable XVS/XHS input\n");
+ //For follower mode, switch both of them to input
+ imx585_write_reg_1byte(imx585, IMX585_REG_XXS_DRV, 0x0F);
+ imx585_write_reg_1byte(imx585, IMX585_REG_XXS_OUTSEL, 0x00);
+ }
+ imx585->common_regs_written = true;
+ dev_info(&client->dev, "common_regs_written\n");
+ }
+
+ /* Apply default values of current mode */
+ reg_list = &imx585->mode->reg_list;
+ ret = imx585_write_regs(imx585, reg_list->regs, reg_list->num_of_regs);
+ if (ret) {
+ dev_err(&client->dev, "%s failed to set mode\n", __func__);
+ return ret;
+ }
+
+ if (imx585->clear_HDR) {
+ ret = imx585_write_regs(imx585, common_clearHDR_mode,
+ ARRAY_SIZE(common_clearHDR_mode));
+ if (ret) {
+ dev_err(&client->dev, "%s failed to set ClearHDR settings\n", __func__);
+ return ret;
+ }
+ //16bit mode is linear, 12bit mode we need to enable gradation compression
+ switch (imx585->fmt_code) {
+ /* 16-bit */
+ case MEDIA_BUS_FMT_SRGGB16_1X16:
+ case MEDIA_BUS_FMT_SGRBG16_1X16:
+ case MEDIA_BUS_FMT_SGBRG16_1X16:
+ case MEDIA_BUS_FMT_SBGGR16_1X16:
+ case MEDIA_BUS_FMT_Y16_1X16:
+ imx585_write_reg_1byte(imx585, IMX595_REG_CCMP_EN, 0);
+ imx585_write_reg_1byte(imx585, 0x3023, 0x03); // MDBIT 16-bit
+ dev_info(&client->dev, "16bit HDR written\n");
+ break;
+ /* 12-bit */
+ case MEDIA_BUS_FMT_SRGGB12_1X12:
+ case MEDIA_BUS_FMT_SGRBG12_1X12:
+ case MEDIA_BUS_FMT_SGBRG12_1X12:
+ case MEDIA_BUS_FMT_SBGGR12_1X12:
+ case MEDIA_BUS_FMT_Y12_1X12:
+ imx585_write_reg_1byte(imx585, IMX595_REG_CCMP_EN, 1);
+ dev_info(&client->dev, "12bit HDR written\n");
+ break;
+ default:
+ break;
+ }
+ dev_info(&client->dev, "ClearHDR_regs_written\n");
+
+ } else {
+ ret = imx585_write_regs(imx585, common_normal_mode, ARRAY_SIZE(common_normal_mode));
+ if (ret) {
+ dev_err(&client->dev, "%s failed to set Normal settings\n", __func__);
+ return ret;
+ }
+ dev_info(&client->dev, "normal_regs_written\n");
+ }
+
+ /* Disable digital clamp */
+ imx585_write_reg_1byte(imx585, IMX585_REG_DIGITAL_CLAMP, 0);
+
+ /* Apply customized values from user */
+ ret = __v4l2_ctrl_handler_setup(imx585->sd.ctrl_handler);
+ if (ret) {
+ dev_err(&client->dev, "%s failed to apply user values\n", __func__);
+ return ret;
+ }
+
+ if (imx585->sync_mode <= 1) {
+ dev_info(&client->dev, "imx585 Leader mode enabled\n");
+ imx585_write_reg_1byte(imx585, IMX585_REG_XMSTA, 0x00);
+ }
+
+ /* Set stream on register */
+ ret = imx585_write_reg_1byte(imx585, IMX585_REG_MODE_SELECT, IMX585_MODE_STREAMING);
+
+ dev_info(&client->dev, "Start Streaming\n");
+ usleep_range(IMX585_STREAM_DELAY_US, IMX585_STREAM_DELAY_US + IMX585_STREAM_DELAY_RANGE_US);
+ return ret;
+}
+
+/* Stop streaming */
+static void imx585_stop_streaming(struct imx585 *imx585)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ int ret;
+
+ dev_info(&client->dev, "Stop Streaming\n");
+
+ /* set stream off register */
+ ret = imx585_write_reg_1byte(imx585, IMX585_REG_MODE_SELECT, IMX585_MODE_STANDBY);
+ if (ret)
+ dev_err(&client->dev, "%s failed to stop stream\n", __func__);
+}
+
+static int imx585_set_stream(struct v4l2_subdev *sd, int enable)
+{
+ struct imx585 *imx585 = to_imx585(sd);
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ int ret = 0;
+
+ mutex_lock(&imx585->mutex);
+ if (imx585->streaming == enable) {
+ mutex_unlock(&imx585->mutex);
+ return 0;
+ }
+
+ if (enable) {
+ ret = pm_runtime_get_sync(&client->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(&client->dev);
+ goto err_unlock;
+ }
+
+ /*
+ * Apply default & customized values
+ * and then start streaming.
+ */
+ ret = imx585_start_streaming(imx585);
+ if (ret)
+ goto err_rpm_put;
+ } else {
+ imx585_stop_streaming(imx585);
+ pm_runtime_put(&client->dev);
+ }
+
+ imx585->streaming = enable;
+
+ /* vflip/hflip and hdr mode cannot change during streaming */
+ __v4l2_ctrl_grab(imx585->vflip, enable);
+ __v4l2_ctrl_grab(imx585->hflip, enable);
+ __v4l2_ctrl_grab(imx585->hdr_mode, enable);
+
+ mutex_unlock(&imx585->mutex);
+
+ return ret;
+
+err_rpm_put:
+ pm_runtime_put(&client->dev);
+err_unlock:
+ mutex_unlock(&imx585->mutex);
+
+ return ret;
+}
+
+/* Power/clock management functions */
+static int imx585_power_on(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx585 *imx585 = to_imx585(sd);
+ int ret;
+
+ ret = regulator_bulk_enable(imx585_NUM_SUPPLIES,
+ imx585->supplies);
+ if (ret) {
+ dev_err(&client->dev, "%s: failed to enable regulators\n",
+ __func__);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(imx585->xclk);
+ if (ret) {
+ dev_err(&client->dev, "%s: failed to enable clock\n",
+ __func__);
+ goto reg_off;
+ }
+
+ gpiod_set_value_cansleep(imx585->reset_gpio, 1);
+ usleep_range(IMX585_XCLR_MIN_DELAY_US,
+ IMX585_XCLR_MIN_DELAY_US + IMX585_XCLR_DELAY_RANGE_US);
+
+ return 0;
+
+reg_off:
+ regulator_bulk_disable(imx585_NUM_SUPPLIES, imx585->supplies);
+ return ret;
+}
+
+static int imx585_power_off(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx585 *imx585 = to_imx585(sd);
+
+ gpiod_set_value_cansleep(imx585->reset_gpio, 0);
+ regulator_bulk_disable(imx585_NUM_SUPPLIES, imx585->supplies);
+ clk_disable_unprepare(imx585->xclk);
+
+ /* Force reprogramming of the common registers when powered up again. */
+ imx585->common_regs_written = false;
+
+ return 0;
+}
+
+static int __maybe_unused imx585_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx585 *imx585 = to_imx585(sd);
+
+ if (imx585->streaming)
+ imx585_stop_streaming(imx585);
+
+ return 0;
+}
+
+static int __maybe_unused imx585_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx585 *imx585 = to_imx585(sd);
+ int ret;
+
+ if (imx585->streaming) {
+ ret = imx585_start_streaming(imx585);
+ if (ret)
+ goto error;
+ }
+
+ return 0;
+
+error:
+ imx585_stop_streaming(imx585);
+ imx585->streaming = 0;
+ return ret;
+}
+
+static int imx585_get_regulators(struct imx585 *imx585)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ unsigned int i;
+
+ for (i = 0; i < imx585_NUM_SUPPLIES; i++)
+ imx585->supplies[i].supply = imx585_supply_name[i];
+
+ return devm_regulator_bulk_get(&client->dev,
+ imx585_NUM_SUPPLIES,
+ imx585->supplies);
+}
+
+/* Verify chip ID */
+static int imx585_check_module_exists(struct imx585 *imx585)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ int ret;
+ u32 val;
+
+ /* We don't actually have a CHIP ID register so we try to read from BLKLEVEL instead*/
+ ret = imx585_read_reg(imx585, IMX585_REG_BLKLEVEL,
+ 1, &val);
+ if (ret) {
+ dev_err(&client->dev, "failed to read chip reg, with error %d\n", ret);
+ return ret;
+ }
+
+ dev_info(&client->dev, "Reg read success, Device found\n");
+
+ return 0;
+}
+
+static int imx585_get_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_selection *sel)
+{
+ switch (sel->target) {
+ case V4L2_SEL_TGT_CROP: {
+ struct imx585 *imx585 = to_imx585(sd);
+
+ mutex_lock(&imx585->mutex);
+ sel->r = *__imx585_get_pad_crop(imx585, sd_state, sel->pad, sel->which);
+ mutex_unlock(&imx585->mutex);
+
+ return 0;
+ }
+
+ case V4L2_SEL_TGT_NATIVE_SIZE:
+ sel->r.left = 0;
+ sel->r.top = 0;
+ sel->r.width = IMX585_NATIVE_WIDTH;
+ sel->r.height = IMX585_NATIVE_HEIGHT;
+ return 0;
+
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ sel->r.left = IMX585_PIXEL_ARRAY_LEFT;
+ sel->r.top = IMX585_PIXEL_ARRAY_TOP;
+ sel->r.width = IMX585_PIXEL_ARRAY_WIDTH;
+ sel->r.height = IMX585_PIXEL_ARRAY_HEIGHT;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static const struct v4l2_subdev_core_ops imx585_core_ops = {
+ .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+ .unsubscribe_event = v4l2_event_subdev_unsubscribe,
+};
+
+static const struct v4l2_subdev_video_ops imx585_video_ops = {
+ .s_stream = imx585_set_stream,
+};
+
+static const struct v4l2_subdev_pad_ops imx585_pad_ops = {
+ .enum_mbus_code = imx585_enum_mbus_code,
+ .get_fmt = imx585_get_pad_format,
+ .set_fmt = imx585_set_pad_format,
+ .get_selection = imx585_get_selection,
+ .enum_frame_size = imx585_enum_frame_size,
+};
+
+static const struct v4l2_subdev_ops imx585_subdev_ops = {
+ .core = &imx585_core_ops,
+ .video = &imx585_video_ops,
+ .pad = &imx585_pad_ops,
+};
+
+static const struct v4l2_subdev_internal_ops imx585_internal_ops = {
+ .open = imx585_open,
+};
+
+/* Initialize control handlers */
+static int imx585_init_controls(struct imx585 *imx585)
+{
+ struct v4l2_ctrl_handler *ctrl_hdlr;
+ struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
+ struct v4l2_fwnode_device_properties props;
+ int ret;
+
+ ctrl_hdlr = &imx585->ctrl_handler;
+ ret = v4l2_ctrl_handler_init(ctrl_hdlr, 32);
+ if (ret)
+ return ret;
+
+ mutex_init(&imx585->mutex);
+ ctrl_hdlr->lock = &imx585->mutex;
+
+ /*
+ * Create the controls here, but mode specific limits are setup
+ * in the imx585_set_framing_limits() call below.
+ */
+ /* By default, PIXEL_RATE is read only */
+ imx585->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
+ V4L2_CID_PIXEL_RATE,
+ 0xffff,
+ 0xffff, 1,
+ 0xffff);
+
+ /* LINK_FREQ is also read only */
+ imx585->link_freq =
+ v4l2_ctrl_new_int_menu(ctrl_hdlr, &imx585_ctrl_ops,
+ V4L2_CID_LINK_FREQ, 0, 0,
+ &link_freqs[imx585->link_freq_idx]);
+ if (imx585->link_freq)
+ imx585->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+ imx585->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
+ V4L2_CID_VBLANK, 0, 0xfffff, 1, 0);
+ imx585->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
+ V4L2_CID_HBLANK, 0, 0xffff, 1, 0);
+ imx585->blacklevel = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
+ V4L2_CID_BRIGHTNESS, 0, 0xffff, 1,
+ IMX585_BLKLEVEL_DEFAULT);
+
+ imx585->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
+ V4L2_CID_EXPOSURE,
+ IMX585_EXPOSURE_MIN,
+ IMX585_EXPOSURE_MAX,
+ IMX585_EXPOSURE_STEP,
+ IMX585_EXPOSURE_DEFAULT);
+
+ imx585->gain = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops, V4L2_CID_ANALOGUE_GAIN,
+ IMX585_ANA_GAIN_MIN_NORMAL, IMX585_ANA_GAIN_MAX_NORMAL,
+ IMX585_ANA_GAIN_STEP, IMX585_ANA_GAIN_DEFAULT);
+
+ imx585->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops, V4L2_CID_HFLIP, 0, 1, 1, 0);
+ imx585->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0);
+
+ if (imx585->has_ircut) {
+ imx585->ircut_ctrl =
+ v4l2_ctrl_new_std(&imx585->ctrl_handler, &imx585_ctrl_ops,
+ V4L2_CID_BAND_STOP_FILTER,
+ 0, 1, 1, 1);
+ }
+
+ imx585->hdr_mode = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
+ V4L2_CID_WIDE_DYNAMIC_RANGE,
+ 0, 1, 1, 0);
+ imx585->datasel_th_ctrl = v4l2_ctrl_new_custom(ctrl_hdlr,
+ &imx585_cfg_datasel_th, NULL);
+ imx585->datasel_bk_ctrl = v4l2_ctrl_new_custom(ctrl_hdlr,
+ &imx585_cfg_datasel_bk, NULL);
+ imx585->gdc_th_ctrl = v4l2_ctrl_new_custom(ctrl_hdlr,
+ &imx585_cfg_grad_th, NULL);
+ imx585->gdc_exp_ctrl_l = v4l2_ctrl_new_custom(ctrl_hdlr,
+ &imx585_cfg_grad_exp_l, NULL);
+ imx585->gdc_exp_ctrl_h = v4l2_ctrl_new_custom(ctrl_hdlr,
+ &imx585_cfg_grad_exp_h, NULL);
+ imx585->hdr_gain_ctrl = v4l2_ctrl_new_custom(ctrl_hdlr,
+ &imx585_cfg_hdr_gain, NULL);
+ imx585->hcg_ctrl = v4l2_ctrl_new_custom(ctrl_hdlr,
+ &imx585_cfg_hcg, NULL);
+
+ v4l2_ctrl_activate(imx585->datasel_th_ctrl, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->datasel_bk_ctrl, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->gdc_th_ctrl, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->gdc_exp_ctrl_l, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->gdc_exp_ctrl_h, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->hdr_gain_ctrl, imx585->clear_HDR);
+ v4l2_ctrl_activate(imx585->hcg_ctrl, !imx585->clear_HDR);
+
+ if (ctrl_hdlr->error) {
+ ret = ctrl_hdlr->error;
+ dev_err(&client->dev, "%s control init failed (%d)\n",
+ __func__, ret);
+ goto error;
+ }
+
+ ret = v4l2_fwnode_device_parse(&client->dev, &props);
+ if (ret)
+ goto error;
+
+ ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &imx585_ctrl_ops, &props);
+ if (ret)
+ goto error;
+
+ memcpy(imx585->datasel_th_ctrl->p_cur.p, hdr_thresh_def, sizeof(hdr_thresh_def));
+ memcpy(imx585->datasel_th_ctrl->p_new.p, hdr_thresh_def, sizeof(hdr_thresh_def));
+ memcpy(imx585->gdc_th_ctrl->p_cur.p, grad_thresh_def, sizeof(grad_thresh_def));
+ memcpy(imx585->gdc_th_ctrl->p_new.p, grad_thresh_def, sizeof(grad_thresh_def));
+
+ imx585->hdr_mode->flags |= V4L2_CTRL_FLAG_UPDATE;
+ imx585->hdr_mode->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
+
+ imx585->sd.ctrl_handler = ctrl_hdlr;
+
+ /* Setup exposure and frame/line length limits. */
+ imx585_set_framing_limits(imx585);
+
+ return 0;
+
+error:
+ v4l2_ctrl_handler_free(ctrl_hdlr);
+ mutex_destroy(&imx585->mutex);
+
+ return ret;
+}
+
+static void imx585_free_controls(struct imx585 *imx585)
+{
+ v4l2_ctrl_handler_free(imx585->sd.ctrl_handler);
+ mutex_destroy(&imx585->mutex);
+}
+
+static const struct of_device_id imx585_dt_ids[] = {
+ { .compatible = "sony,imx585"},
+ { /* sentinel */ }
+};
+
+static int imx585_check_hwcfg(struct device *dev, struct imx585 *imx585)
+{
+ struct fwnode_handle *endpoint;
+ struct v4l2_fwnode_endpoint ep_cfg = {
+ .bus_type = V4L2_MBUS_CSI2_DPHY
+ };
+ int ret = -EINVAL;
+ int i;
+
+ endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
+ if (!endpoint) {
+ dev_err(dev, "endpoint node not found\n");
+ return -EINVAL;
+ }
+
+ if (v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep_cfg)) {
+ dev_err(dev, "could not parse endpoint\n");
+ goto error_out;
+ }
+
+ /* Check the number of MIPI CSI2 data lanes */
+ if (ep_cfg.bus.mipi_csi2.num_data_lanes != 2 && ep_cfg.bus.mipi_csi2.num_data_lanes != 4) {
+ dev_err(dev, "only 2 or 4 data lanes are currently supported\n");
+ goto error_out;
+ }
+ imx585->lane_count = ep_cfg.bus.mipi_csi2.num_data_lanes;
+ dev_info(dev, "Data lanes: %d\n", imx585->lane_count);
+
+ /* Check the link frequency set in device tree */
+ if (!ep_cfg.nr_of_link_frequencies) {
+ dev_err(dev, "link-frequency property not found in DT\n");
+ goto error_out;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(link_freqs); i++) {
+ if (link_freqs[i] == ep_cfg.link_frequencies[0]) {
+ imx585->link_freq_idx = i;
+ break;
+ }
+ }
+
+ if (i == ARRAY_SIZE(link_freqs)) {
+ dev_err(dev, "Link frequency not supported: %lld\n",
+ ep_cfg.link_frequencies[0]);
+ ret = -EINVAL;
+ goto error_out;
+ }
+
+ dev_info(dev, "Link Speed: %lld Mhz\n", ep_cfg.link_frequencies[0]);
+
+ ret = 0;
+
+error_out:
+ v4l2_fwnode_endpoint_free(&ep_cfg);
+ fwnode_handle_put(endpoint);
+
+ return ret;
+}
+
+static int imx585_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct device_node *np;
+ struct imx585 *imx585;
+ const struct of_device_id *match;
+ int ret, i;
+ u32 sync_mode;
+
+ imx585 = devm_kzalloc(&client->dev, sizeof(*imx585), GFP_KERNEL);
+ if (!imx585)
+ return -ENOMEM;
+
+ v4l2_i2c_subdev_init(&imx585->sd, client, &imx585_subdev_ops);
+
+ match = of_match_device(imx585_dt_ids, dev);
+ if (!match)
+ return -ENODEV;
+
+ dev_info(dev, "Reading dtoverlay config:\n");
+ imx585->mono = of_property_read_bool(dev->of_node, "mono-mode");
+ if (imx585->mono)
+ dev_info(dev, "Mono Mode Selected, make sure you have the correct sensor variant\n");
+
+ imx585->clear_HDR = of_property_read_bool(dev->of_node, "clearHDR-mode");
+ dev_info(dev, "ClearHDR: %d\n", imx585->clear_HDR);
+
+ imx585->sync_mode = 0;
+ ret = of_property_read_u32(dev->of_node, "sync-mode", &sync_mode);
+ if (!ret) {
+ if (sync_mode > 2) {
+ dev_warn(dev, "sync-mode out of range, using 0\n");
+ sync_mode = 0;
+ }
+ imx585->sync_mode = sync_mode;
+ } else if (ret != -EINVAL) { /* property present but bad */
+ dev_err(dev, "sync-mode malformed (%pe)\n", ERR_PTR(ret));
+ return ret;
+ }
+ dev_info(dev, "Sync Mode: %s\n", sync_mode_menu[imx585->sync_mode]);
+
+ /* Check the hardware configuration in device tree */
+ if (imx585_check_hwcfg(dev, imx585))
+ return -EINVAL;
+
+ /* Get system clock (xclk) */
+ imx585->xclk = devm_clk_get(dev, NULL);
+ if (IS_ERR(imx585->xclk)) {
+ dev_err(dev, "failed to get xclk\n");
+ return PTR_ERR(imx585->xclk);
+ }
+
+ imx585->xclk_freq = clk_get_rate(imx585->xclk);
+
+ for (i = 0; i < ARRAY_SIZE(imx585_inck_table); ++i) {
+ if (imx585_inck_table[i].xclk_hz == imx585->xclk_freq) {
+ imx585->inck_sel_val = imx585_inck_table[i].inck_sel;
+ break;
+ }
+ }
+
+ if (i == ARRAY_SIZE(imx585_inck_table)) {
+ dev_err(dev, "unsupported XCLK rate %u Hz\n",
+ imx585->xclk_freq);
+ return -EINVAL;
+ }
+
+ dev_info(dev, "XCLK %u Hz → INCK_SEL 0x%02x\n",
+ imx585->xclk_freq, imx585->inck_sel_val);
+
+ ret = imx585_get_regulators(imx585);
+ if (ret) {
+ dev_err(dev, "failed to get regulators\n");
+ return ret;
+ }
+
+ /* Request optional enable pin */
+ imx585->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+ GPIOD_OUT_HIGH);
+
+ /*
+ * The sensor must be powered for imx585_check_module_exists()
+ * to be able to read register
+ */
+ ret = imx585_power_on(dev);
+ if (ret)
+ return ret;
+
+ ret = imx585_check_module_exists(imx585);
+ if (ret)
+ goto error_power_off;
+
+ imx585->has_ircut = false;
+ imx585->ircut_client = NULL;
+
+ if (of_property_read_bool(dev->of_node, "ircut-mode")) {
+ np = of_parse_phandle(dev->of_node, "ircut-controller", 0);
+ if (np) {
+ imx585->ircut_client = of_find_i2c_device_by_node(np);
+ of_node_put(np);
+ ret = imx585_ircut_write(imx585, 0x01);
+ if (!ret) {
+ imx585->has_ircut = true;
+ dev_info(dev, "IR-cut controller present at 0x%02x\n",
+ imx585->ircut_client->addr);
+ } else {
+ dev_info(dev,
+ "Can't communication with IR-cut control, dropping\n");
+ }
+ }
+ } else {
+ dev_info(dev, "No IR-cut controller\n");
+ }
+
+ /* Initialize default format */
+ imx585_set_default_format(imx585);
+
+ /* Enable runtime PM and turn off the device */
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ pm_runtime_idle(dev);
+
+ /* This needs the pm runtime to be registered. */
+ ret = imx585_init_controls(imx585);
+ if (ret)
+ goto error_pm_runtime;
+
+ /* Initialize subdev */
+ imx585->sd.internal_ops = &imx585_internal_ops;
+ imx585->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
+ V4L2_SUBDEV_FL_HAS_EVENTS;
+ imx585->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+
+ /* Initialize source pads */
+ imx585->pad[IMAGE_PAD].flags = MEDIA_PAD_FL_SOURCE;
+ imx585->pad[METADATA_PAD].flags = MEDIA_PAD_FL_SOURCE;
+
+ ret = media_entity_pads_init(&imx585->sd.entity, NUM_PADS, imx585->pad);
+ if (ret) {
+ dev_err(dev, "failed to init entity pads: %d\n", ret);
+ goto error_handler_free;
+ }
+
+ ret = v4l2_async_register_subdev_sensor(&imx585->sd);
+ if (ret < 0) {
+ dev_err(dev, "failed to register sensor sub-device: %d\n", ret);
+ goto error_media_entity;
+ }
+
+ return 0;
+
+error_media_entity:
+ media_entity_cleanup(&imx585->sd.entity);
+
+error_handler_free:
+ imx585_free_controls(imx585);
+
+error_pm_runtime:
+ pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+
+error_power_off:
+ imx585_power_off(&client->dev);
+
+ return ret;
+}
+
+static void imx585_remove(struct i2c_client *client)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx585 *imx585 = to_imx585(sd);
+
+ v4l2_async_unregister_subdev(sd);
+ media_entity_cleanup(&sd->entity);
+ imx585_free_controls(imx585);
+
+ pm_runtime_disable(&client->dev);
+ if (!pm_runtime_status_suspended(&client->dev))
+ imx585_power_off(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+}
+
+MODULE_DEVICE_TABLE(of, imx585_dt_ids);
+
+static const struct dev_pm_ops imx585_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(imx585_suspend, imx585_resume)
+ SET_RUNTIME_PM_OPS(imx585_power_off, imx585_power_on, NULL)
+};
+
+static struct i2c_driver imx585_i2c_driver = {
+ .driver = {
+ .name = "imx585",
+ .of_match_table = imx585_dt_ids,
+ .pm = &imx585_pm_ops,
+ },
+ .probe = imx585_probe,
+ .remove = imx585_remove,
+};
+
+module_i2c_driver(imx585_i2c_driver);
+
+MODULE_AUTHOR("Will Whang <will@willwhang.com>");
+MODULE_AUTHOR("Tetsuya NOMURA <tetsuya.nomura@soho-enterprise.com>");
+MODULE_DESCRIPTION("Sony imx585 sensor driver");
+MODULE_LICENSE("GPL");
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 4/4] media: docs: Add userspace-API guide for the IMX585 driver
2025-07-02 6:38 [PATCH v1 0/4] media: Add Sony IMX585 image sensor support Will Whang
` (2 preceding siblings ...)
2025-07-02 6:38 ` [PATCH v1 3/4] media: i2c: imx585: Add Sony IMX585 image-sensor driver Will Whang
@ 2025-07-02 6:38 ` Will Whang
3 siblings, 0 replies; 12+ messages in thread
From: Will Whang @ 2025-07-02 6:38 UTC (permalink / raw)
To: Will Whang, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
The new IMX585 V4L2 sub-device driver introduces several
driver-specific controls for configuring Clear-HDR blending,
gradation compression thresholds, and HCG enabling. This patch adds
an rst document under Documentation/userspace-api/media/drivers/
that details each control, allowed values, and their effect
Signed-off-by: Will Whang <will@willwhang.com>
---
.../userspace-api/media/drivers/imx585.rst | 95 +++++++++++++++++++
1 file changed, 95 insertions(+)
create mode 100644 Documentation/userspace-api/media/drivers/imx585.rst
diff --git a/Documentation/userspace-api/media/drivers/imx585.rst b/Documentation/userspace-api/media/drivers/imx585.rst
new file mode 100644
index 000000000..bb08afa93
--- /dev/null
+++ b/Documentation/userspace-api/media/drivers/imx585.rst
@@ -0,0 +1,95 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Sony IMX585 driver
+==================
+
+The IMX585 image-sensor driver provides the following *driver-specific*
+V4L2 controls. They are visible only when the IMX585 driver is loaded
+and sit in the sensor-private control class.
+
+HDR data blending
+-----------------
+
+``V4L2_CID_IMX585_HDR_DATASEL_TH`` (``U16[2]``)
+ Lower/upper **thresholds** (0 – 4095) that decide which exposure is
+ chosen—or blended—for each pixel in Clear-HDR mode.
+
+``V4L2_CID_IMX585_HDR_DATASEL_BK`` (menu)
+ **Blending ratio** between the long-gain (LG) and
+ high-gain (HG) read-outs.
+
+ .. flat-table::
+ :stub-columns: 0
+ :widths: 1 5
+
+ * - ``0`` - HG ½, LG ½
+ * - ``1`` - HG ¾, LG ¼
+ * - ``2`` - HG ½, LG ½ *(duplicate ratio present in the
+ datasheet)*
+ * - ``3`` - HG ⅞, LG ⅛
+ * - ``4`` - HG 15⁄16, LG 1⁄16
+ * - ``5`` - **2ⁿᵈ** HG ½, LG ½ *(second 50 %-50 % entry as
+ documented)*
+ * - ``6`` - HG 1⁄16, LG 15⁄16
+ * - ``7`` - HG ⅛, LG ⅞
+ * - ``8`` - HG ¼, LG ¾
+
+Gradation compression
+---------------------
+
+``V4L2_CID_IMX585_HDR_GRAD_TH`` (``U32[2]``)
+ 17-bit **break-points** (0 – 0x1ffff) that shape the 16-bit
+ gradation-compression curve.
+
+``V4L2_CID_IMX585_HDR_GRAD_COMP_L`` (menu)
+``V4L2_CID_IMX585_HDR_GRAD_COMP_H`` (menu)
+ **Compression ratios** below the first break-point and between the
+ two break-points, respectively.
+
+ .. flat-table::
+ :stub-columns: 0
+ :widths: 1 4
+
+ * - ``0`` - 1 : 1
+ * - ``1`` - 1 : 2
+ * - ``2`` - 1 : 4 *(default for COMP_L)*
+ * - ``3`` - 1 : 8
+ * - ``4`` - 1 : 16
+ * - ``5`` - 1 : 32
+ * - ``6`` - 1 : 64 *(default for COMP_H)*
+ * - ``7`` - 1 : 128
+ * - ``8`` - 1 : 256
+ * - ``9`` - 1 : 512
+ * - ``10`` - 1 : 1024
+ * - ``11`` - 1 : 2048
+
+Gain settings
+-------------
+
+``V4L2_CID_IMX585_HDR_GAIN`` (menu)
+ **Additional gain** (in dB) applied to the high-gain path when
+ Clear-HDR is active.
+
+ .. flat-table::
+ :stub-columns: 0
+ :widths: 1 3
+
+ * - ``0`` - +0 dB
+ * - ``1`` - +6 dB
+ * - ``2`` - +12 dB *(default)*
+ * - ``3`` - +18 dB
+ * - ``4`` - +24 dB
+ * - ``5`` - +29.1 dB
+
+``V4L2_CID_IMX585_HCG_GAIN`` (boolean)
+ Toggle **High-Conversion-Gain** mode.
+
+ *0 = LCG (default), 1 = HCG.*
+
+Notes
+-----
+
+* Controls are writable while streaming; changes take effect from the
+ next frame.
+* HDR-specific controls are hidden when HDR is disabled.
+* Inter-control dependencies are enforced by the driver.
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
2025-07-02 6:38 ` [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor Will Whang
@ 2025-07-02 9:28 ` Laurent Pinchart
[not found] ` <CAFoNnrxquDp_yx_HSOe00cVDMcw2G+rTZs8x8RgOD3RO=tq-XA@mail.gmail.com>
2025-07-04 8:08 ` Krzysztof Kozlowski
1 sibling, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2025-07-02 9:28 UTC (permalink / raw)
To: Will Whang
Cc: Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Will,
Thank you for the patch.
On Wed, Jul 02, 2025 at 07:38:33AM +0100, Will Whang wrote:
> Document the devicetree binding for the Sony IMX585. The schema
> covers the CSI-2 data-lanes, the optional 'mono-mode' flag,
> and the internal-sync properties used by the driver.
>
> Signed-off-by: Will Whang <will@willwhang.com>
> ---
> .../bindings/media/i2c/sony,imx585.yaml | 120 ++++++++++++++++++
> MAINTAINERS | 8 ++
> 2 files changed, 128 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> new file mode 100644
> index 000000000..d050d1642
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> @@ -0,0 +1,120 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2024 Ideas on Board Oy
Unless there's something I'm not aware of, I don't think Ideas on Board
wrote this. You can use your own copyright.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/sony,imx585.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony IMX585 Sensor
> +
> +maintainers:
> + - Will Whang <will@willwhang.com>
> +
> +description:
> + IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
> +
You should add
allOf:
- $ref: /schemas/media/video-interface-devices.yaml#
here to support generic sensor properties. You will need to replace
additionalProperties: false
with
unevaluatedProperties: false
below.
> +properties:
> + compatible:
> + const: sony,imx585
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: xclk
When there's a single clock you can drop clock-names.
> +
> + clock-frequency:
> + enum: [ 74250000, 37125000, 72000000, 27000000, 24000000 ]
The clock-frequency property is frowned upon for sensors in DT. If the
aim is to set the frequency of the clock, it should be done through
assigned-clocks and assigned-clock-rates. If the aim is to convey the
clock frequency to the driver, it should be done by calling
clk_get_rate() in the driver.
> +
> + reg:
> + maxItems: 1
> + description: I2C Address for IMX585
You can drop the description, it's always the same for I2C devices.
> +
> + VANA-supply:
> + description: Analog power supply (3.3V)
> +
> + VDDL-supply:
> + description: Interface power supply (1.8V)
> +
> + VDIG-supply:
> + description: Digital power supply (1.1V)
> +
> + reset-gpios:
> + description: Sensor reset (XCLR) GPIO
> + maxItems: 1
> +
> + port:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + additionalProperties: false
> +
> + properties:
> + endpoint:
> + $ref: /schemas/media/video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + data-lanes:
> + anyOf:
> + - items:
> + - const: 1
> + - const: 2
> + - const: 3
> + - const: 4
Does that mean that the sensor supports data lane remapping ? I don't
see it implemented by the driver. If it's not supported by the hardware,
you should use
properties:
data-lanes:
minItems: 1
items:
- const: 1
- const: 2
- const: 3
- const: 4
To guarantee the order.
> +
> + sync-mode:
> + description: |
> + Select the synchronisation mode of the sensor
> + 0 – internal sync, leader (default)
> + 1 – internal sync, follower
> + 2 – external sync
> + $ref: /schemas/types.yaml#/definitions/uint8
> + enum: [ 0, 1, 2 ]
This seems to be a sensor-level property, not an endpoint property. As
it's not standard, it should also have a vendor prefix, i.e.
sony,sync-mode. I'm wondering, though, if we shouldn't try to
standardize it in video-interface-devices.yaml.
> +
> + link-frequencies:
> + description: Select the MIPI-CSI2 link speed in Mhz
You can drop the description, it's already described in
video-interfaces.yaml.
> + items:
> + enum: [ 297000000, 360000000, 445500000, 594000000,
> + 720000000, 891000000, 1039500000 ]
Are those frequencies the only ones the hardware can support, or do they
come from the driver only supporting a fixed set of sensor PLL
configurations ? In the latter case I would drop the enumeration.
> +
> + required:
> + - data-lanes
> + - link-frequencies
> +
> + required:
> + - endpoint
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-frequency
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + imx585@1a {
> + compatible = "sony,imx585";
> + reg = <0x1a>;
> + clocks = <&imx585_clk>;
> + clock-frequency = <24000000>;
> +
> + VANA-supply = <&camera_vadd_3v3>;
> + VDDL-supply = <&camera_vdd1_1v8>;
> + VDIG-supply = <&camera_vdd2_1v1>;
> +
> + port {
> + imx585: endpoint {
> + remote-endpoint = <&cam>;
> + data-lanes = <1 2 3 4>;
> + link-frequencies = /bits/ 64 <720000000>;
> + };
> + };
> + };
> + };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index da34c7227..9cc404790 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23150,6 +23150,14 @@ T: git git://linuxtv.org/media.git
> F: Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
> F: drivers/media/i2c/imx415.c
>
> +SONY IMX585 SENSOR DRIVER
> +M: Will Whang <will@willwhang.com>
> +L: linux-media@vger.kernel.org
> +S: Maintained
> +T: git git://linuxtv.org/media.git
> +F: Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> +F: drivers/media/i2c/imx585.c
> +
> SONY MEMORYSTICK SUBSYSTEM
> M: Maxim Levitsky <maximlevitsky@gmail.com>
> M: Alex Dubov <oakad@yahoo.com>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 3/4] media: i2c: imx585: Add Sony IMX585 image-sensor driver
2025-07-02 6:38 ` [PATCH v1 3/4] media: i2c: imx585: Add Sony IMX585 image-sensor driver Will Whang
@ 2025-07-03 17:51 ` Laurent Pinchart
2025-07-03 17:54 ` Laurent Pinchart
[not found] ` <CAFoNnrx-YpQwY6_908x=8LK1uwWw0y5zKxsv+aTsW1fxX554vg@mail.gmail.com>
0 siblings, 2 replies; 12+ messages in thread
From: Laurent Pinchart @ 2025-07-03 17:51 UTC (permalink / raw)
To: Will Whang
Cc: Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Will,
Thank you for the patch.
Here's a first review, focussing on API usage and coding style.
On Wed, Jul 02, 2025 at 07:38:35AM +0100, Will Whang wrote:
> Implements support for:
> * 4-lane / 2-lane CSI-2
> * 12-bit linear, 12-bit HDR-GC and 16-bit Clear-HDR modes
> * Mono variant switch, HCG, custom HDR controls
> * Tested on Raspberry Pi 4/5 with 24 MHz XCLK.
>
> Signed-off-by: Will Whang <will@willwhang.com>
> ---
> drivers/media/i2c/Kconfig | 9 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/imx585.c | 2466 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 2476 insertions(+)
> create mode 100644 drivers/media/i2c/imx585.c
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index e68202954..34eb1c19a 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -266,6 +266,15 @@ config VIDEO_IMX415
> To compile this driver as a module, choose M here: the
> module will be called imx415.
>
> +config VIDEO_IMX585
> + tristate "Sony IMX585 sensor support"
> + help
> + This is a Video4Linux2 sensor driver for the Sony
> + IMX585 camera.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called imx585.
> +
> config VIDEO_MAX9271_LIB
> tristate
>
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 5873d2943..887d19ca7 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -59,6 +59,7 @@ obj-$(CONFIG_VIDEO_IMX335) += imx335.o
> obj-$(CONFIG_VIDEO_IMX355) += imx355.o
> obj-$(CONFIG_VIDEO_IMX412) += imx412.o
> obj-$(CONFIG_VIDEO_IMX415) += imx415.o
> +obj-$(CONFIG_VIDEO_IMX585) += imx585.o
> obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
> obj-$(CONFIG_VIDEO_ISL7998X) += isl7998x.o
> obj-$(CONFIG_VIDEO_KS0127) += ks0127.o
> diff --git a/drivers/media/i2c/imx585.c b/drivers/media/i2c/imx585.c
> new file mode 100644
> index 000000000..2c4212290
> --- /dev/null
> +++ b/drivers/media/i2c/imx585.c
> @@ -0,0 +1,2466 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * A V4L2 driver for Sony imx585 cameras.
> + *
> + * Based on Sony imx477 camera driver
> + * Copyright (C) 2019-2020 Raspberry Pi (Trading) Ltd
> + * Modified by Will WHANG
> + * Modified by sohonomura2020 in Soho Enterprise Ltd.
> + */
Please add a blank line here.
> +#include <linux/unaligned.h>
And move this lower to keep include statements sorted alphabetically.
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_graph.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mediabus.h>
> +
> +// Support for rpi kernel pre git commit 314a685
We yse C-style comments only. Please update the comment style through
the driver.
> +#ifndef MEDIA_BUS_FMT_SENSOR_DATA
> +#define MEDIA_BUS_FMT_SENSOR_DATA 0x7002
> +#endif
That's for the downstream Raspberry Pi kernel, but is not applicable to
upstream. We're working on an upstream API for embedded data, see
https://lore.kernel.org/linux-media/20250619115836.1946016-1-sakari.ailus@linux.intel.com/.
I recommend dropping embedded data support first, as the API isn't
available upstream yet, and adding it back as a separate patch.
> +
> +#define V4L2_CID_IMX585_HDR_DATASEL_TH (V4L2_CID_USER_IMX585_BASE + 0)
> +#define V4L2_CID_IMX585_HDR_DATASEL_BK (V4L2_CID_USER_IMX585_BASE + 1)
> +#define V4L2_CID_IMX585_HDR_GRAD_TH (V4L2_CID_USER_IMX585_BASE + 2)
> +#define V4L2_CID_IMX585_HDR_GRAD_COMP_L (V4L2_CID_USER_IMX585_BASE + 3)
> +#define V4L2_CID_IMX585_HDR_GRAD_COMP_H (V4L2_CID_USER_IMX585_BASE + 4)
> +#define V4L2_CID_IMX585_HDR_GAIN (V4L2_CID_USER_IMX585_BASE + 5)
> +#define V4L2_CID_IMX585_HCG_GAIN (V4L2_CID_USER_IMX585_BASE + 6)
> +
> +/* Standby or streaming mode */
> +#define IMX585_REG_MODE_SELECT 0x3000
> +#define IMX585_MODE_STANDBY 0x01
> +#define IMX585_MODE_STREAMING 0x00
> +#define IMX585_STREAM_DELAY_US 25000
> +#define IMX585_STREAM_DELAY_RANGE_US 1000
> +
> +/*
> + * Initialisation delay between XCLR low->high and the moment when the sensor
> + * can start capture (i.e. can leave software standby)
> + */
> +#define IMX585_XCLR_MIN_DELAY_US 500000
> +#define IMX585_XCLR_DELAY_RANGE_US 1000
> +
> +/* Leader mode and XVS/XHS direction */
> +#define IMX585_REG_XMSTA 0x3002
> +#define IMX585_REG_XXS_DRV 0x30A6
Please use lower-case for hex constants.
> +#define IMX585_REG_EXTMODE 0x30CE
> +#define IMX585_REG_XXS_OUTSEL 0x30A4
> +
> +/*XVS pulse length, 2^n H with n=0~3*/
Missing space after /* and before */. Please update other locations as
appropriate.
> +#define IMX585_REG_XVSLNG 0x30CC
> +/*XHS pulse length, 16*(2^n) Clock with n=0~3*/
> +#define IMX585_REG_XHSLNG 0x30CD
> +
> +/* Clk selection */
> +#define IMX585_INCK_SEL 0x3014
> +
> +/* Link Speed */
> +#define IMX585_DATARATE_SEL 0x3015
> +
> +/* BIN mode */
> +/* 2x2 Bin mode selection, 0x01 => Mono, 0x00 => Color */
> +#define IMX585_BIN_MODE 0x3019
> +
> +/* Lane Count */
> +#define IMX585_LANEMODE 0x3040
> +
> +/* VMAX internal VBLANK*/
> +#define IMX585_REG_VMAX 0x3028
> +#define IMX585_VMAX_MAX 0xfffff
> +#define IMX585_VMAX_DEFAULT 2250
> +
> +/* HMAX internal HBLANK*/
> +#define IMX585_REG_HMAX 0x302C
> +#define IMX585_HMAX_MAX 0xffff
> +
> +/* SHR internal */
> +#define IMX585_REG_SHR 0x3050
> +#define IMX585_SHR_MIN 8
> +#define IMX585_SHR_MIN_HDR 10
> +#define IMX585_SHR_MAX 0xfffff
> +
> +/* Exposure control */
> +#define IMX585_EXPOSURE_MIN 2
> +#define IMX585_EXPOSURE_STEP 1
> +#define IMX585_EXPOSURE_DEFAULT 1000
> +#define IMX585_EXPOSURE_MAX 49865
> +
> +/* HDR threshold */
> +#define IMX585_REG_EXP_TH_H 0x36D0
> +#define IMX585_REG_EXP_TH_L 0x36D4
> +#define IMX585_REG_EXP_BK 0x36E2
> +
> +/* Gradation compression control */
> +#define IMX595_REG_CCMP_EN 0x36EF
> +#define IMX585_REG_CCMP1_EXP 0x36E8
> +#define IMX585_REG_CCMP2_EXP 0x36E4
> +#define IMX585_REG_ACMP1_EXP 0x36EE
> +#define IMX585_REG_ACMP2_EXP 0x36EC
> +
> +/* HDR Gain Adder */
> +#define IMX585_REG_EXP_GAIN 0x3081
> +
> +/* Black level control */
> +#define IMX585_REG_BLKLEVEL 0x30DC
> +#define IMX585_BLKLEVEL_DEFAULT 50
> +
> +/* Digital Clamp */
> +#define IMX585_REG_DIGITAL_CLAMP 0x3458
> +
> +/* Analog gain control */
> +#define IMX585_REG_ANALOG_GAIN 0x306C
> +#define IMX585_REG_FDG_SEL0 0x3030
> +#define IMX585_ANA_GAIN_MIN_NORMAL 0
> +#define IMX585_ANA_GAIN_MIN_HCG 34
> +#define IMX585_ANA_GAIN_MAX_HDR 80
> +#define IMX585_ANA_GAIN_MAX_NORMAL 240
> +#define IMX585_ANA_GAIN_STEP 1
> +#define IMX585_ANA_GAIN_DEFAULT 0
> +
> +/* Flip */
> +#define IMX585_FLIP_WINMODEH 0x3020
> +#define IMX585_FLIP_WINMODEV 0x3021
> +
> +/* Embedded metadata stream structure */
> +#define IMX585_EMBEDDED_LINE_WIDTH 16384
> +#define IMX585_NUM_EMBEDDED_LINES 1
> +
> +#define IMX585_PIXEL_RATE 74250000
> +
> +enum pad_types {
> + IMAGE_PAD,
> + METADATA_PAD,
> + NUM_PADS
> +};
> +
> +/* imx585 native and active pixel array size. */
> +#define IMX585_NATIVE_WIDTH 3856U
> +#define IMX585_NATIVE_HEIGHT 2180U
> +#define IMX585_PIXEL_ARRAY_LEFT 8U
> +#define IMX585_PIXEL_ARRAY_TOP 8U
> +#define IMX585_PIXEL_ARRAY_WIDTH 3840U
> +#define IMX585_PIXEL_ARRAY_HEIGHT 2160U
> +
> +/* Link frequency setup */
> +enum {
> + IMX585_LINK_FREQ_297MHZ, // 594Mbps/lane
> + IMX585_LINK_FREQ_360MHZ, // 720Mbps/lane
> + IMX585_LINK_FREQ_445MHZ, // 891Mbps/lane
> + IMX585_LINK_FREQ_594MHZ, // 1188Mbps/lane
> + IMX585_LINK_FREQ_720MHZ, // 1440Mbps/lane
> + IMX585_LINK_FREQ_891MHZ, // 1782Mbps/lane
> + IMX585_LINK_FREQ_1039MHZ, // 2079Mbps/lane
> + IMX585_LINK_FREQ_1188MHZ, // 2376Mbps/lane
> +};
> +
> +static const u8 link_freqs_reg_value[] = {
> + [IMX585_LINK_FREQ_297MHZ] = 0x07,
> + [IMX585_LINK_FREQ_360MHZ] = 0x06,
> + [IMX585_LINK_FREQ_445MHZ] = 0x05,
> + [IMX585_LINK_FREQ_594MHZ] = 0x04,
> + [IMX585_LINK_FREQ_720MHZ] = 0x03,
> + [IMX585_LINK_FREQ_891MHZ] = 0x02,
> + [IMX585_LINK_FREQ_1039MHZ] = 0x01,
> + [IMX585_LINK_FREQ_1188MHZ] = 0x00,
> +};
> +
> +static const u64 link_freqs[] = {
> + [IMX585_LINK_FREQ_297MHZ] = 297000000,
> + [IMX585_LINK_FREQ_360MHZ] = 360000000,
> + [IMX585_LINK_FREQ_445MHZ] = 445500000,
> + [IMX585_LINK_FREQ_594MHZ] = 594000000,
> + [IMX585_LINK_FREQ_720MHZ] = 720000000,
> + [IMX585_LINK_FREQ_891MHZ] = 891000000,
> + [IMX585_LINK_FREQ_1039MHZ] = 1039500000,
> + [IMX585_LINK_FREQ_1188MHZ] = 1188000000,
> +};
> +
> +//min HMAX for 4-lane 4K full res mode, x2 for 2-lane, /2 for FHD
> +static const u16 HMAX_table_4lane_4K[] = {
> + [IMX585_LINK_FREQ_297MHZ] = 1584,
> + [IMX585_LINK_FREQ_360MHZ] = 1320,
> + [IMX585_LINK_FREQ_445MHZ] = 1100,
> + [IMX585_LINK_FREQ_594MHZ] = 792,
> + [IMX585_LINK_FREQ_720MHZ] = 660,
> + [IMX585_LINK_FREQ_891MHZ] = 550,
> + [IMX585_LINK_FREQ_1039MHZ] = 440,
> + [IMX585_LINK_FREQ_1188MHZ] = 396,
> +};
> +
> +struct imx585_inck_cfg {
> + u32 xclk_hz; /* platform clock rate */
> + u8 inck_sel; /* value for reg */
> +};
> +
> +static const struct imx585_inck_cfg imx585_inck_table[] = {
> + { 74250000, 0x00 },
> + { 37125000, 0x01 },
> + { 72000000, 0x02 },
> + { 27000000, 0x03 },
> + { 24000000, 0x04 },
> +};
> +
> +static const char * const hdr_gain_adder_menu[] = {
> + "+0dB",
> + "+6dB",
> + "+12dB",
> + "+18dB",
> + "+24dB",
> + "+29.1dB",
> +};
> +
> +/*Honestly I don't know why there are two 50% 50% blend
> + * but it is in the datasheet
> + */
Multi-line comments should have the comment opening alone on the first
line:
/*
* Honestly I don't know why there are two 50% 50% blend
* but it is in the datasheet
*/
> +static const char * const hdr_data_blender_menu[] = {
> + "HG 1/2, LG 1/2",
> + "HG 3/4, LG 1/4",
> + "HG 1/2, LG 1/2",
> + "HG 7/8, LG 1/8",
> + "HG 15/16, LG 1/16",
> + "2nd HG 1/2, LG 1/2",
> + "HG 1/16, LG 15/16",
> + "HG 1/8, LG 7/8",
> + "HG 1/4, LG 3/4",
> +};
> +
> +static const char * const grad_compression_slope_menu[] = {
> + "1/1",
> + "1/2",
> + "1/4",
> + "1/8",
> + "1/16",
> + "1/32",
> + "1/64",
> + "1/128",
> + "1/256",
> + "1/512",
> + "1/1024",
> + "1/2048",
> +};
> +
> +static const char * const sync_mode_menu[] = {
> + "Internal Sync Leader Mode",
> + "External Sync Leader Mode",
> + "Follower Mode",
> +};
> +
> +struct imx585_reg {
> + u16 address;
> + u8 val;
> +};
> +
> +struct IMX585_reg_list {
> + unsigned int num_of_regs;
> + const struct imx585_reg *regs;
> +};
> +
> +/* Mode : resolution and related config&values */
> +struct imx585_mode {
> + /* Frame width */
> + unsigned int width;
> +
> + /* Frame height */
> + unsigned int height;
> +
> + /* mode HMAX Scaling */
> + u8 hmax_div;
> +
> + /* minimum H-timing */
> + u16 min_HMAX;
> +
> + /* minimum V-timing */
> + u64 min_VMAX;
> +
> + /* default H-timing */
> + u16 default_HMAX;
> +
> + /* default V-timing */
> + u64 default_VMAX;
> +
> + /* Analog crop rectangle. */
> + struct v4l2_rect crop;
> +
> + /* Default register values */
> + struct IMX585_reg_list reg_list;
> +};
> +
> +/* IMX585 Register List */
> +/* Common Modes */
> +static struct imx585_reg common_regs[] = {
> + {0x3002, 0x01},
> + {0x3069, 0x00},
> + {0x3074, 0x64},
> + {0x30D5, 0x04},// DIG_CLP_VSTART
> + {0x3030, 0x00},// FDG_SEL0 LCG, HCG:0x01
> + {0x30A6, 0x00},// XVS_DRV [1:0] Hi-Z
> + {0x3081, 0x00},// EXP_GAIN, Reset to 0
> + {0x3460, 0x21},// -
> + {0x3478, 0xA1},// -
> + {0x347C, 0x01},// -
> + {0x3480, 0x01},// -
> + {0x3A4E, 0x14},// -
> + {0x3A52, 0x14},// -
> + {0x3A56, 0x00},// -
> + {0x3A5A, 0x00},// -
> + {0x3A5E, 0x00},// -
> + {0x3A62, 0x00},// -
> + {0x3A6A, 0x20},// -
> + {0x3A6C, 0x42},// -
> + {0x3A6E, 0xA0},// -
> + {0x3B2C, 0x0C},// -
> + {0x3B30, 0x1C},// -
> + {0x3B34, 0x0C},// -
> + {0x3B38, 0x1C},// -
> + {0x3BA0, 0x0C},// -
> + {0x3BA4, 0x1C},// -
> + {0x3BA8, 0x0C},// -
> + {0x3BAC, 0x1C},// -
> + {0x3D3C, 0x11},// -
> + {0x3D46, 0x0B},// -
> + {0x3DE0, 0x3F},// -
> + {0x3DE1, 0x08},// -
> + {0x3E14, 0x87},// -
> + {0x3E16, 0x91},// -
> + {0x3E18, 0x91},// -
> + {0x3E1A, 0x87},// -
> + {0x3E1C, 0x78},// -
> + {0x3E1E, 0x50},// -
> + {0x3E20, 0x50},// -
> + {0x3E22, 0x50},// -
> + {0x3E24, 0x87},// -
> + {0x3E26, 0x91},// -
> + {0x3E28, 0x91},// -
> + {0x3E2A, 0x87},// -
> + {0x3E2C, 0x78},// -
> + {0x3E2E, 0x50},// -
> + {0x3E30, 0x50},// -
> + {0x3E32, 0x50},// -
> + {0x3E34, 0x87},// -
> + {0x3E36, 0x91},// -
> + {0x3E38, 0x91},// -
> + {0x3E3A, 0x87},// -
> + {0x3E3C, 0x78},// -
> + {0x3E3E, 0x50},// -
> + {0x3E40, 0x50},// -
> + {0x3E42, 0x50},// -
> + {0x4054, 0x64},// -
> + {0x4148, 0xFE},// -
> + {0x4149, 0x05},// -
> + {0x414A, 0xFF},// -
> + {0x414B, 0x05},// -
> + {0x420A, 0x03},// -
> + {0x4231, 0x08},// -
> + {0x423D, 0x9C},// -
> + {0x4242, 0xB4},// -
> + {0x4246, 0xB4},// -
> + {0x424E, 0xB4},// -
> + {0x425C, 0xB4},// -
> + {0x425E, 0xB6},// -
> + {0x426C, 0xB4},// -
> + {0x426E, 0xB6},// -
> + {0x428C, 0xB4},// -
> + {0x428E, 0xB6},// -
> + {0x4708, 0x00},// -
> + {0x4709, 0x00},// -
> + {0x470A, 0xFF},// -
> + {0x470B, 0x03},// -
> + {0x470C, 0x00},// -
> + {0x470D, 0x00},// -
> + {0x470E, 0xFF},// -
> + {0x470F, 0x03},// -
> + {0x47EB, 0x1C},// -
> + {0x47F0, 0xA6},// -
> + {0x47F2, 0xA6},// -
> + {0x47F4, 0xA0},// -
> + {0x47F6, 0x96},// -
> + {0x4808, 0xA6},// -
> + {0x480A, 0xA6},// -
> + {0x480C, 0xA0},// -
> + {0x480E, 0x96},// -
> + {0x492C, 0xB2},// -
> + {0x4930, 0x03},// -
> + {0x4932, 0x03},// -
> + {0x4936, 0x5B},// -
> + {0x4938, 0x82},// -
> + {0x493E, 0x23},// -
> + {0x4BA8, 0x1C},// -
> + {0x4BA9, 0x03},// -
> + {0x4BAC, 0x1C},// -
> + {0x4BAD, 0x1C},// -
> + {0x4BAE, 0x1C},// -
> + {0x4BAF, 0x1C},// -
> + {0x4BB0, 0x1C},// -
> + {0x4BB1, 0x1C},// -
> + {0x4BB2, 0x1C},// -
> + {0x4BB3, 0x1C},// -
> + {0x4BB4, 0x1C},// -
> + {0x4BB8, 0x03},// -
> + {0x4BB9, 0x03},// -
> + {0x4BBA, 0x03},// -
> + {0x4BBB, 0x03},// -
> + {0x4BBC, 0x03},// -
> + {0x4BBD, 0x03},// -
> + {0x4BBE, 0x03},// -
> + {0x4BBF, 0x03},// -
> + {0x4BC0, 0x03},// -
> + {0x4C14, 0x87},// -
> + {0x4C16, 0x91},// -
> + {0x4C18, 0x91},// -
> + {0x4C1A, 0x87},// -
> + {0x4C1C, 0x78},// -
> + {0x4C1E, 0x50},// -
> + {0x4C20, 0x50},// -
> + {0x4C22, 0x50},// -
> + {0x4C24, 0x87},// -
> + {0x4C26, 0x91},// -
> + {0x4C28, 0x91},// -
> + {0x4C2A, 0x87},// -
> + {0x4C2C, 0x78},// -
> + {0x4C2E, 0x50},// -
> + {0x4C30, 0x50},// -
> + {0x4C32, 0x50},// -
> + {0x4C34, 0x87},// -
> + {0x4C36, 0x91},// -
> + {0x4C38, 0x91},// -
> + {0x4C3A, 0x87},// -
> + {0x4C3C, 0x78},// -
> + {0x4C3E, 0x50},// -
> + {0x4C40, 0x50},// -
> + {0x4C42, 0x50},// -
> + {0x4D12, 0x1F},// -
> + {0x4D13, 0x1E},// -
> + {0x4D26, 0x33},// -
> + {0x4E0E, 0x59},// -
> + {0x4E14, 0x55},// -
> + {0x4E16, 0x59},// -
> + {0x4E1E, 0x3B},// -
> + {0x4E20, 0x47},// -
> + {0x4E22, 0x54},// -
> + {0x4E26, 0x81},// -
> + {0x4E2C, 0x7D},// -
> + {0x4E2E, 0x81},// -
> + {0x4E36, 0x63},// -
> + {0x4E38, 0x6F},// -
> + {0x4E3A, 0x7C},// -
> + {0x4F3A, 0x3C},// -
> + {0x4F3C, 0x46},// -
> + {0x4F3E, 0x59},// -
> + {0x4F42, 0x64},// -
> + {0x4F44, 0x6E},// -
> + {0x4F46, 0x81},// -
> + {0x4F4A, 0x82},// -
> + {0x4F5A, 0x81},// -
> + {0x4F62, 0xAA},// -
> + {0x4F72, 0xA9},// -
> + {0x4F78, 0x36},// -
> + {0x4F7A, 0x41},// -
> + {0x4F7C, 0x61},// -
> + {0x4F7D, 0x01},// -
> + {0x4F7E, 0x7C},// -
> + {0x4F7F, 0x01},// -
> + {0x4F80, 0x77},// -
> + {0x4F82, 0x7B},// -
> + {0x4F88, 0x37},// -
> + {0x4F8A, 0x40},// -
> + {0x4F8C, 0x62},// -
> + {0x4F8D, 0x01},// -
> + {0x4F8E, 0x76},// -
> + {0x4F8F, 0x01},// -
> + {0x4F90, 0x5E},// -
> + {0x4F91, 0x02},// -
> + {0x4F92, 0x69},// -
> + {0x4F93, 0x02},// -
> + {0x4F94, 0x89},// -
> + {0x4F95, 0x02},// -
> + {0x4F96, 0xA4},// -
> + {0x4F97, 0x02},// -
> + {0x4F98, 0x9F},// -
> + {0x4F99, 0x02},// -
> + {0x4F9A, 0xA3},// -
> + {0x4F9B, 0x02},// -
> + {0x4FA0, 0x5F},// -
> + {0x4FA1, 0x02},// -
> + {0x4FA2, 0x68},// -
> + {0x4FA3, 0x02},// -
> + {0x4FA4, 0x8A},// -
> + {0x4FA5, 0x02},// -
> + {0x4FA6, 0x9E},// -
> + {0x4FA7, 0x02},// -
> + {0x519E, 0x79},// -
> + {0x51A6, 0xA1},// -
> + {0x51F0, 0xAC},// -
> + {0x51F2, 0xAA},// -
> + {0x51F4, 0xA5},// -
> + {0x51F6, 0xA0},// -
> + {0x5200, 0x9B},// -
> + {0x5202, 0x91},// -
> + {0x5204, 0x87},// -
> + {0x5206, 0x82},// -
> + {0x5208, 0xAC},// -
> + {0x520A, 0xAA},// -
> + {0x520C, 0xA5},// -
> + {0x520E, 0xA0},// -
> + {0x5210, 0x9B},// -
> + {0x5212, 0x91},// -
> + {0x5214, 0x87},// -
> + {0x5216, 0x82},// -
> + {0x5218, 0xAC},// -
> + {0x521A, 0xAA},// -
> + {0x521C, 0xA5},// -
> + {0x521E, 0xA0},// -
> + {0x5220, 0x9B},// -
> + {0x5222, 0x91},// -
> + {0x5224, 0x87},// -
> + {0x5226, 0x82},// -
> +};
> +
> +/* Common Registers for ClearHDR. */
> +static const struct imx585_reg common_clearHDR_mode[] = {
> + {0x301A, 0x10}, // WDMODE: Clear HDR mode
> + {0x3024, 0x02}, // COMBI_EN: 0x02
> + {0x3069, 0x02}, // Clear HDR mode
> + {0x3074, 0x63}, // Clear HDR mode
> + {0x3930, 0xE6}, // DUR[15:8]: Clear HDR mode (12bit)
> + {0x3931, 0x00}, // DUR[7:0]: Clear HDR mode (12bit)
> + {0x3A4C, 0x61}, // WAIT_ST0[7:0]: Clear HDR mode
> + {0x3A4D, 0x02}, // WAIT_ST0[15:8]: Clear HDR mode
> + {0x3A50, 0x70}, // WAIT_ST1[7:0]: Clear HDR mode
> + {0x3A51, 0x02}, // WAIT_ST1[15:8]: Clear HDR mode
> + {0x3E10, 0x17}, // ADTHEN: Clear HDR mode
> + {0x493C, 0x41}, // WAIT_10_SHF AD 10-bit 0x0C disable
> + {0x4940, 0x41}, // WAIT_12_SHF AD 12-bit 0x41 enable
> + {0x3081, 0x02}, // EXP_GAIN: High gain setting +12dB default
> +};
> +
> +/* Common Registers for non-ClearHDR. */
> +static const struct imx585_reg common_normal_mode[] = {
> + {0x301A, 0x00}, // WDMODE: Normal mode
> + {0x3024, 0x00}, // COMBI_EN: 0x00
> + {0x3069, 0x00}, // Normal mode
> + {0x3074, 0x64}, // Normal mode
> + {0x3930, 0x0C}, // DUR[15:8]: Normal mode (12bit)
> + {0x3931, 0x01}, // DUR[7:0]: Normal mode (12bit)
> + {0x3A4C, 0x39}, // WAIT_ST0[7:0]: Normal mode
> + {0x3A4D, 0x01}, // WAIT_ST0[15:8]: Normal mode
> + {0x3A50, 0x48}, // WAIT_ST1[7:0]: Normal mode
> + {0x3A51, 0x01}, // WAIT_ST1[15:8]: Normal mode
> + {0x3E10, 0x10}, // ADTHEN: Normal mode
> + {0x493C, 0x23}, // WAIT_10_SHF AD 10-bit 0x23 Normal mode
> + {0x4940, 0x23}, // WAIT_12_SHF AD 12-bit 0x23 Normal mode
> +};
> +
> +/* All pixel 4K60. 12-bit */
> +static const struct imx585_reg mode_4k_regs_12bit[] = {
> + {0x301B, 0x00}, // ADDMODE non-binning
> + {0x3022, 0x02}, // ADBIT 12-bit
> + {0x3023, 0x01}, // MDBIT 12-bit
> + {0x30D5, 0x04}, // DIG_CLP_VSTART non-binning
> +};
> +
> +/* 2x2 binned 1080p60. 12-bit */
> +static const struct imx585_reg mode_1080_regs_12bit[] = {
> + {0x301B, 0x01}, // ADDMODE binning
> + {0x3022, 0x02}, // ADBIT 12-bit
> + {0x3023, 0x01}, // MDBIT 12-bit
> + {0x30D5, 0x02}, // DIG_CLP_VSTART binning
> +};
> +
> +/* IMX585 Register List - END*/
> +
> +/* For Mode List:
> + * Default:
> + * 12Bit - FHD, 4K
> + * ClearHDR Enabled:
> + * 12bit + Gradation compression
> + * 16bit - FHD, 4K
> + *
> + * Gradation compression is available on 12bit
> + * With Default option, only 12bit mode is exposed
> + * With ClearHDR enabled via parameters,
> + * 12bit will be with Gradation compression enabled
> + * 16bit mode exposed
> + *
> + * Technically, because the sensor is actually binning
> + * in digital domain, its readout speed is the same
> + * between 4K and FHD. However, through testing it is
> + * possible to "overclock" the FHD mode, thus leaving the
> + * hmax_div option for those who want to try.
> + * Also, note that FHD and 4K mode shared the same VMAX.
> + */
> +
> +/* Mode configs */
> +struct imx585_mode supported_modes[] = {
> + {
> + /* 1080p60 2x2 binning */
> + .width = 1928,
> + .height = 1090,
> + .hmax_div = 1,
> + .min_HMAX = 366,
> + .min_VMAX = IMX585_VMAX_DEFAULT,
> + .default_HMAX = 366,
> + .default_VMAX = IMX585_VMAX_DEFAULT,
> + .crop = {
> + .left = IMX585_PIXEL_ARRAY_LEFT,
> + .top = IMX585_PIXEL_ARRAY_TOP,
> + .width = IMX585_PIXEL_ARRAY_WIDTH,
> + .height = IMX585_PIXEL_ARRAY_HEIGHT,
> + },
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mode_1080_regs_12bit),
> + .regs = mode_1080_regs_12bit,
> + },
> + },
> + {
> + /* 4K60 All pixel */
> + .width = 3856,
> + .height = 2180,
> + .min_HMAX = 550,
> + .min_VMAX = IMX585_VMAX_DEFAULT,
> + .default_HMAX = 550,
> + .default_VMAX = IMX585_VMAX_DEFAULT,
> + .hmax_div = 1,
> + .crop = {
> + .left = IMX585_PIXEL_ARRAY_LEFT,
> + .top = IMX585_PIXEL_ARRAY_TOP,
> + .width = IMX585_PIXEL_ARRAY_WIDTH,
> + .height = IMX585_PIXEL_ARRAY_HEIGHT,
> + },
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mode_4k_regs_12bit),
> + .regs = mode_4k_regs_12bit,
> + },
> + },
> +};
> +
> +/*
> + * The supported formats.
> + * This table MUST contain 4 entries per format, to cover the various flip
> + * combinations in the order
> + * - no flip
> + * - h flip
> + * - v flip
> + * - h&v flips
> + */
> +
> +/* 12bit Only */
> +static const u32 codes_normal[] = {
> + MEDIA_BUS_FMT_SRGGB12_1X12,
> + MEDIA_BUS_FMT_SGRBG12_1X12,
> + MEDIA_BUS_FMT_SGBRG12_1X12,
> + MEDIA_BUS_FMT_SBGGR12_1X12,
> +};
> +
> +/* 12bit + 16bit for Clear HDR */
> +static const u32 codes_clearhdr[] = {
> + /* 16-bit modes. */
> + MEDIA_BUS_FMT_SRGGB16_1X16,
> + MEDIA_BUS_FMT_SGRBG16_1X16,
> + MEDIA_BUS_FMT_SGBRG16_1X16,
> + MEDIA_BUS_FMT_SBGGR16_1X16,
> + /* 12-bit modes. */
> + MEDIA_BUS_FMT_SRGGB12_1X12,
> + MEDIA_BUS_FMT_SGRBG12_1X12,
> + MEDIA_BUS_FMT_SGBRG12_1X12,
> + MEDIA_BUS_FMT_SBGGR12_1X12,
> +};
> +
> +/* Flip isn’t relevant for mono */
> +static const u32 mono_codes[] = {
> + MEDIA_BUS_FMT_Y16_1X16, /* 16-bit mono */
> + MEDIA_BUS_FMT_Y12_1X12, /* 12-bit mono */
> +};
> +
> +/* regulator supplies */
> +static const char * const imx585_supply_name[] = {
> + /* Supplies can be enabled in any order */
> + "VANA", /* Analog (3.3V) supply */
> + "VDIG", /* Digital Core (1.1V) supply */
> + "VDDL", /* IF (1.8V) supply */
> +};
> +
> +#define imx585_NUM_SUPPLIES ARRAY_SIZE(imx585_supply_name)
> +
> +struct imx585 {
> + struct v4l2_subdev sd;
> + struct media_pad pad[NUM_PADS];
> +
> + unsigned int fmt_code;
Please use the v4l2_subdev active state API to replace this. You will
need to call v4l2_subdev_init_finalize(), and the V4L2 framework will
allocate a v4l2_subdev_state instance for the active state. Most subdev
operations won't have to test for V4L2_SUBDEV_FORMAT_ACTIVE or
V4L2_SUBDEV_FORMAT_TRY, and will simply store data in the state passed
to the function. See commit e8a5b1df000e ("media: i2c: imx219: Use
subdev active state") for an example.
> +
> + struct clk *xclk;
> + u32 xclk_freq;
> +
> + /* chosen INCK_SEL register value */
> + u8 inck_sel_val;
> +
> + /* Link configurations */
> + unsigned int lane_count;
> + unsigned int link_freq_idx;
> +
> + struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data supplies[imx585_NUM_SUPPLIES];
> +
> + struct v4l2_ctrl_handler ctrl_handler;
> +
> + /* V4L2 Controls */
> + struct v4l2_ctrl *pixel_rate;
> + struct v4l2_ctrl *link_freq;
> + struct v4l2_ctrl *exposure;
> + struct v4l2_ctrl *gain;
> + struct v4l2_ctrl *hcg_ctrl;
> + struct v4l2_ctrl *vflip;
> + struct v4l2_ctrl *hflip;
> + struct v4l2_ctrl *vblank;
> + struct v4l2_ctrl *hblank;
> + struct v4l2_ctrl *blacklevel;
> +
> + /* V4L2 HDR Controls */
> + struct v4l2_ctrl *hdr_mode;
> + struct v4l2_ctrl *datasel_th_ctrl;
> + struct v4l2_ctrl *datasel_bk_ctrl;
> + struct v4l2_ctrl *gdc_th_ctrl;
> + struct v4l2_ctrl *gdc_exp_ctrl_l;
> + struct v4l2_ctrl *gdc_exp_ctrl_h;
> + struct v4l2_ctrl *hdr_gain_ctrl;
> +
> + /* V4L2 IR Cut filter switch Controls */
> + bool has_ircut;
> + struct v4l2_ctrl *ircut_ctrl;
> + struct i2c_client *ircut_client;
> +
> + /* Current mode */
> + const struct imx585_mode *mode;
This should be dropped too, the imx585_mode instance should be lookup up
based on the v4l2_subdev_state.
> +
> + /* HCG enabled flag*/
> + bool hcg;
> +
> + /* Mono mode */
> + bool mono;
> +
> + /* Clear HDR mode */
> + bool clear_HDR;
All variables must be lower case.
> +
> + /* Sync Mode*/
> + /* 0 = Internal Sync Leader Mode
> + * 1 = External Sync Leader Mode
> + * 2 = Follower Mode
> + * The datasheet wording is very confusing but basically:
> + * Leader Mode = Sensor using internal clock to drive the sensor
> + * But with external sync mode you can send a XVS input so the sensor
> + * will try to align with it.
> + * For Follower mode it is purely driven by external clock.
> + * In this case you need to drive both XVS and XHS.
> + */
> + u32 sync_mode;
> +
> + /* Tracking sensor VMAX/HMAX value */
> + u16 HMAX;
> + u32 VMAX;
> +
> + /*
> + * Mutex for serialized access:
> + * Protect sensor module set pad format and start/stop streaming safely.
> + */
> + struct mutex mutex;
You will be able to drop this when switching to the v4l2_subdev active
state API.
> +
> + /* Streaming on/off */
> + bool streaming;
> +
> + /* Rewrite common registers on stream on? */
> + bool common_regs_written;
> +};
> +
> +static inline struct imx585 *to_imx585(struct v4l2_subdev *_sd)
> +{
> + return container_of(_sd, struct imx585, sd);
> +}
> +
> +static inline void get_mode_table(struct imx585 *imx585, unsigned int code,
> + const struct imx585_mode **mode_list,
> + unsigned int *num_modes)
> +{
> + *mode_list = NULL;
> + *num_modes = 0;
> +
> + if (imx585->mono) {
> + /* --- Mono paths --- */
> + if (code == MEDIA_BUS_FMT_Y16_1X16 && imx585->clear_HDR) {
> + *mode_list = supported_modes;
> + *num_modes = ARRAY_SIZE(supported_modes);
> + }
> + if (code == MEDIA_BUS_FMT_Y12_1X12) {
> + *mode_list = supported_modes;
> + *num_modes = ARRAY_SIZE(supported_modes);
> + }
> + } else {
> + /* --- Color paths --- */
> + switch (code) {
> + /* 16-bit */
> + case MEDIA_BUS_FMT_SRGGB16_1X16:
> + case MEDIA_BUS_FMT_SGRBG16_1X16:
> + case MEDIA_BUS_FMT_SGBRG16_1X16:
> + case MEDIA_BUS_FMT_SBGGR16_1X16:
> + /* 12-bit */
> + case MEDIA_BUS_FMT_SRGGB12_1X12:
> + case MEDIA_BUS_FMT_SGRBG12_1X12:
> + case MEDIA_BUS_FMT_SGBRG12_1X12:
> + case MEDIA_BUS_FMT_SBGGR12_1X12:
> + *mode_list = supported_modes;
> + *num_modes = ARRAY_SIZE(supported_modes);
> + break;
> + default:
> + *mode_list = NULL;
> + *num_modes = 0;
> + }
> + }
> +}
> +
> +/* ------------------------------------------------------------------
> + * Optional IR-cut helper
> + * ------------------------------------------------------------------
> + */
> +
> +/* One-byte “command” sent to the IR-cut MCU at imx585->ircut_client */
Is that MCU integrated in the camera sensor, or in the camera module ?
> +static int imx585_ircut_write(struct imx585 *imx585, u8 cmd)
> +{
> + struct i2c_client *client = imx585->ircut_client;
> + int ret;
> +
> + ret = i2c_smbus_write_byte(client, cmd);
> + if (ret < 0)
> + dev_err(&client->dev, "IR-cut write failed (%d)\n", ret);
> +
> + return ret;
> +}
> +
> +static int imx585_ircut_set(struct imx585 *imx585, int on)
> +{
> + return imx585_ircut_write(imx585, on ? 0x01 : 0x00);
> +}
> +
> +/* Read registers up to 2 at a time */
> +static int imx585_read_reg(struct imx585 *imx585, u16 reg, u32 len, u32 *val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + struct i2c_msg msgs[2];
> + u8 addr_buf[2] = { reg >> 8, reg & 0xff };
> + u8 data_buf[4] = { 0, };
> + int ret;
> +
> + if (len > 4)
> + return -EINVAL;
> +
> + /* Write register address */
> + msgs[0].addr = client->addr;
> + msgs[0].flags = 0;
> + msgs[0].len = ARRAY_SIZE(addr_buf);
> + msgs[0].buf = addr_buf;
> +
> + /* Read data from register */
> + msgs[1].addr = client->addr;
> + msgs[1].flags = I2C_M_RD;
> + msgs[1].len = len;
> + msgs[1].buf = &data_buf[4 - len];
> +
> + ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (ret != ARRAY_SIZE(msgs))
> + return -EIO;
> +
> + *val = get_unaligned_be32(data_buf);
> +
> + return 0;
> +}
> +
> +/* Write registers 1 byte at a time */
> +static int imx585_write_reg_1byte(struct imx585 *imx585, u16 reg, u8 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + u8 buf[3];
> + int ret;
> +
> + put_unaligned_be16(reg, buf);
> + buf[2] = val;
> + ret = i2c_master_send(client, buf, 3);
> + if (ret != 3)
> + return ret;
> +
> + return 0;
> +}
> +
> +/* Write registers 2 byte at a time */
> +static int imx585_write_reg_2byte(struct imx585 *imx585, u16 reg, u16 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + u8 buf[4];
> + int ret;
> +
> + put_unaligned_be16(reg, buf);
> + buf[2] = val;
> + buf[3] = val >> 8;
> + ret = i2c_master_send(client, buf, 4);
> + if (ret != 4)
> + return ret;
> +
> + return 0;
> +}
> +
> +/* Write registers 3 byte at a time */
> +static int imx585_write_reg_3byte(struct imx585 *imx585, u16 reg, u32 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + u8 buf[5];
> +
> + put_unaligned_be16(reg, buf);
> + buf[2] = val;
> + buf[3] = val >> 8;
> + buf[4] = val >> 16;
> + if (i2c_master_send(client, buf, 5) != 5)
> + return -EIO;
> +
> + return 0;
> +}
Please use the v4l2-cci helpers to access registers. They encode the
register width in the register address macro, simplifying the driver and
making it less error-prone.
> +
> +/* Write a list of 1 byte registers */
> +static int imx585_write_regs(struct imx585 *imx585,
> + const struct imx585_reg *regs, u32 len)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < len; i++) {
> + ret = imx585_write_reg_1byte(imx585, regs[i].address,
> + regs[i].val);
> + if (ret) {
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + regs[i].address, ret);
> +
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/* Hold register values until hold is disabled */
> +static inline void imx585_register_hold(struct imx585 *imx585, bool hold)
> +{
> + imx585_write_reg_1byte(imx585, 0x3001, hold ? 1 : 0);
> +}
> +
> +/* Get bayer order based on flip setting. */
> +static u32 imx585_get_format_code(struct imx585 *imx585, u32 code)
> +{
> + unsigned int i;
> +
> + lockdep_assert_held(&imx585->mutex);
> +
> + if (imx585->mono) {
> + for (i = 0; i < ARRAY_SIZE(mono_codes); i++)
> + if (mono_codes[i] == code)
> + break;
> + return mono_codes[i];
> + }
> +
> + if (imx585->clear_HDR) {
> + for (i = 0; i < ARRAY_SIZE(codes_clearhdr); i++)
> + if (codes_clearhdr[i] == code)
> + break;
> + return codes_clearhdr[i];
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(codes_normal); i++)
> + if (codes_normal[i] == code)
> + break;
> + return codes_normal[i];
> +}
> +
> +static void imx585_set_default_format(struct imx585 *imx585)
> +{
> + /* Set default mode to max resolution */
> + imx585->mode = &supported_modes[0];
> + if (imx585->mono)
> + imx585->fmt_code = MEDIA_BUS_FMT_Y12_1X12;
> + else
> + imx585->fmt_code = MEDIA_BUS_FMT_SRGGB12_1X12;
> +}
> +
> +static int imx585_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
This should be dropped and replaced by the .init_state() operation.
> +{
> + struct imx585 *imx585 = to_imx585(sd);
> + struct v4l2_mbus_framefmt *try_fmt_img =
> + v4l2_subdev_state_get_format(fh->state, IMAGE_PAD);
> + struct v4l2_mbus_framefmt *try_fmt_meta =
> + v4l2_subdev_state_get_format(fh->state, METADATA_PAD);
> + struct v4l2_rect *try_crop;
> +
> + mutex_lock(&imx585->mutex);
> +
> + /* Initialize try_fmt for the image pad */
> + try_fmt_img->width = supported_modes[0].width;
> + try_fmt_img->height = supported_modes[0].height;
> + if (imx585->mono)
> + try_fmt_img->code = imx585_get_format_code(imx585, MEDIA_BUS_FMT_Y12_1X12);
> + else
> + try_fmt_img->code = imx585_get_format_code(imx585, MEDIA_BUS_FMT_SRGGB12_1X12);
> +
> + try_fmt_img->field = V4L2_FIELD_NONE;
> +
> + /* Initialize try_fmt for the embedded metadata pad */
> + try_fmt_meta->width = IMX585_EMBEDDED_LINE_WIDTH;
> + try_fmt_meta->height = IMX585_NUM_EMBEDDED_LINES;
> + try_fmt_meta->code = MEDIA_BUS_FMT_SENSOR_DATA;
> + try_fmt_meta->field = V4L2_FIELD_NONE;
> +
> + /* Initialize try_crop */
> + try_crop = v4l2_subdev_state_get_crop(fh->state, IMAGE_PAD);
> + try_crop->left = IMX585_PIXEL_ARRAY_LEFT;
> + try_crop->top = IMX585_PIXEL_ARRAY_TOP;
> + try_crop->width = IMX585_PIXEL_ARRAY_WIDTH;
> + try_crop->height = IMX585_PIXEL_ARRAY_HEIGHT;
> +
> + mutex_unlock(&imx585->mutex);
> +
> + return 0;
> +}
> +
> +/* For HDR mode, Gain is limited to 0~80 and HCG is disabled
> + * For Normal mode, Gain is limited to 0~240
> + */
> +static void imx585_update_gain_limits(struct imx585 *imx585)
> +{
> + bool hcg_on = imx585->hcg;
Wrong indentation.
> + bool clear_hdr = imx585->clear_HDR;
> + u32 min = hcg_on ? IMX585_ANA_GAIN_MIN_HCG : IMX585_ANA_GAIN_MIN_NORMAL;
> + u32 max = clear_hdr ? IMX585_ANA_GAIN_MAX_HDR : IMX585_ANA_GAIN_MAX_NORMAL;
> + u32 cur = imx585->gain->val;
> +
> + __v4l2_ctrl_modify_range(imx585->gain,
> + min, max,
> + IMX585_ANA_GAIN_STEP,
> + clamp(cur, min, max));
> +
> + if (cur < min || cur > max)
> + __v4l2_ctrl_s_ctrl(imx585->gain,
> + clamp(cur, min, max));
> +}
> +
> +static void imx585_update_hmax(struct imx585 *imx585)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
Store the struct device pointer in struct imx585 and use it instead of
client->dev. This should drop most usages of i2c_client from the driver.
> +
> + const u32 base_4lane = HMAX_table_4lane_4K[imx585->link_freq_idx];
> + const u32 lane_scale = (imx585->lane_count == 2) ? 2 : 1;
> + const u32 factor = base_4lane * lane_scale;
> + const u32 hdr_factor = (imx585->clear_HDR) ? 2 : 1;
> +
> + dev_info(&client->dev, "Upadte minimum HMAX\n");
> + dev_info(&client->dev, "\tbase_4lane: %d\n", base_4lane);
> + dev_info(&client->dev, "\tlane_scale: %d\n", lane_scale);
> + dev_info(&client->dev, "\tfactor: %d\n", factor);
> + dev_info(&client->dev, "\thdr_factor: %d\n", hdr_factor);
This makes the driver way too chatty. The messages should be demoted to
dev_dbg(), or dropped.
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(supported_modes); ++i) {
> + u32 h = factor / supported_modes[i].hmax_div;
> + u64 v = IMX585_VMAX_DEFAULT * hdr_factor;
> +
> + supported_modes[i].min_HMAX = h;
> + supported_modes[i].default_HMAX = h;
> + supported_modes[i].min_VMAX = v;
> + supported_modes[i].default_VMAX = v;
> + dev_info(&client->dev, "\tvmax: %lld x hmax: %d\n", v, h);
> + }
> +}
> +
> +static void imx585_set_framing_limits(struct imx585 *imx585)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + const struct imx585_mode *mode = imx585->mode;
> + u64 default_hblank, max_hblank;
> + u64 pixel_rate;
> +
> + imx585_update_hmax(imx585);
> +
> + dev_info(&client->dev, "mode: %d x %d\n", mode->width, mode->height);
> +
> + imx585->VMAX = mode->default_VMAX;
> + imx585->HMAX = mode->default_HMAX;
> +
> + pixel_rate = (u64)mode->width * IMX585_PIXEL_RATE;
> + /* In the case where ClearHDR is enabled, HMAX is effectly doubled */
> + /* So pixel rate is half with the same HMAX with ClearHDR */
> + do_div(pixel_rate, mode->min_HMAX);
> + __v4l2_ctrl_modify_range(imx585->pixel_rate, pixel_rate, pixel_rate, 1, pixel_rate);
> +
> + //int default_hblank = mode->default_HMAX*IMX585_PIXEL_RATE/72000000-IMX585_NATIVE_WIDTH;
> + default_hblank = mode->default_HMAX * pixel_rate;
> + do_div(default_hblank, IMX585_PIXEL_RATE);
> + default_hblank = default_hblank - mode->width;
> +
> + max_hblank = IMX585_HMAX_MAX * pixel_rate;
> + do_div(max_hblank, IMX585_PIXEL_RATE);
> + max_hblank = max_hblank - mode->width;
> +
> + __v4l2_ctrl_modify_range(imx585->hblank, 0, max_hblank, 1, default_hblank);
> + __v4l2_ctrl_s_ctrl(imx585->hblank, default_hblank);
> +
> + /* Update limits and set FPS to default */
> + __v4l2_ctrl_modify_range(imx585->vblank, mode->min_VMAX - mode->height,
> + IMX585_VMAX_MAX - mode->height,
> + 1, mode->default_VMAX - mode->height);
> + __v4l2_ctrl_s_ctrl(imx585->vblank, mode->default_VMAX - mode->height);
> +
> + __v4l2_ctrl_modify_range(imx585->exposure, IMX585_EXPOSURE_MIN,
> + imx585->VMAX - IMX585_SHR_MIN_HDR, 1,
> + IMX585_EXPOSURE_DEFAULT);
> + dev_info(&client->dev, "default vmax: %lld x hmax: %d\n", mode->min_VMAX, mode->min_HMAX);
> + dev_info(&client->dev, "Setting default HBLANK : %llu, VBLANK : %llu PixelRate: %lld\n",
> + default_hblank, mode->default_VMAX - mode->height, pixel_rate);
> +}
> +
> +static int imx585_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct imx585 *imx585 = container_of(ctrl->handler, struct imx585, ctrl_handler);
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + const struct imx585_mode *mode = imx585->mode;
> + const struct imx585_mode *mode_list;
> + unsigned int code, num_modes;
> +
> + int ret = 0;
> + /*
> + * Applying V4L2 control value that
> + * doesn't need to be in streaming mode
> + */
> + switch (ctrl->id) {
> + case V4L2_CID_WIDE_DYNAMIC_RANGE:
> + if (imx585->clear_HDR != ctrl->val) {
> + imx585->clear_HDR = ctrl->val;
> + v4l2_ctrl_activate(imx585->datasel_th_ctrl, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->datasel_bk_ctrl, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->gdc_th_ctrl, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->gdc_exp_ctrl_h, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->gdc_exp_ctrl_l, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->hdr_gain_ctrl, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->hcg_ctrl, !imx585->clear_HDR);
> + imx585_update_gain_limits(imx585);
> + if (imx585->mono)
> + code = imx585_get_format_code(imx585, MEDIA_BUS_FMT_Y12_1X12);
> + else
> + code = imx585_get_format_code(imx585, MEDIA_BUS_FMT_SRGGB12_1X12);
> + get_mode_table(imx585, code, &mode_list, &num_modes);
> + imx585->mode = v4l2_find_nearest_size(mode_list,
> + num_modes,
> + width, height,
> + imx585->mode->width,
> + imx585->mode->height);
> + imx585_set_framing_limits(imx585);
> + }
> + break;
> + }
> +
> + /*
> + * Applying V4L2 control value only happens
> + * when power is up for streaming
> + */
> + if (pm_runtime_get_if_in_use(&client->dev) == 0)
> + return 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_EXPOSURE:
> + {
> + u32 shr;
case V4L2_CID_EXPOSURE: {
u32 shr;
Same below.
> +
> + shr = (imx585->VMAX - ctrl->val) & ~1u; //Always a multiple of 2
> + dev_info(&client->dev, "V4L2_CID_EXPOSURE : %d\n", ctrl->val);
> + dev_info(&client->dev, "\tVMAX:%d, HMAX:%d\n", imx585->VMAX, imx585->HMAX);
> + dev_info(&client->dev, "\tSHR:%d\n", shr);
> +
> + ret = imx585_write_reg_3byte(imx585, IMX585_REG_SHR, shr);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_SHR, ret);
> + break;
> + }
> + case V4L2_CID_IMX585_HCG_GAIN:
> + {
> + if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE)
> + break;
> + imx585->hcg = ctrl->val;
> + imx585_update_gain_limits(imx585);
> +
> + // Set HCG/LCG channel
> + ret = imx585_write_reg_1byte(imx585, IMX585_REG_FDG_SEL0, ctrl->val);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_FDG_SEL0, ret);
> + dev_info(&client->dev, "V4L2_CID_HCG_ENABLE: %d\n", ctrl->val);
> + break;
> + }
> + case V4L2_CID_ANALOGUE_GAIN:
> + {
> + u32 gain = ctrl->val;
> +
> + dev_info(&client->dev, "analogue gain = %u (%s)\n",
> + gain, imx585->hcg ? "HCG" : "LCG");
> +
> + ret = imx585_write_reg_2byte(imx585, IMX585_REG_ANALOG_GAIN, gain);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "ANALOG_GAIN write failed (%d)\n", ret);
> + break;
> + }
> + case V4L2_CID_VBLANK:
> + {
> + u32 current_exposure = imx585->exposure->cur.val;
> + u32 min_SHR = (imx585->clear_HDR) ? IMX585_SHR_MIN_HDR : IMX585_SHR_MIN;
> + /*
> + * The VBLANK control may change the limits of usable exposure, so check
> + * and adjust if necessary.
> + */
> + imx585->VMAX = (mode->height + ctrl->val) & ~1u; //Always a multiple of 2
> +
> + /* New maximum exposure limits,
> + * modifying the range and make sure we are not exceed the new maximum.
> + */
> + current_exposure = clamp_t(u32, current_exposure, IMX585_EXPOSURE_MIN,
> + imx585->VMAX - min_SHR);
> + __v4l2_ctrl_modify_range(imx585->exposure, IMX585_EXPOSURE_MIN,
> + imx585->VMAX - min_SHR, 1,
> + current_exposure);
> +
> + dev_info(&client->dev, "V4L2_CID_VBLANK : %d\n", ctrl->val);
> + dev_info(&client->dev, "\tVMAX:%d, HMAX:%d\n", imx585->VMAX, imx585->HMAX);
> + dev_info(&client->dev, "Update exposure limits: max:%d, min:%d, current:%d\n",
> + imx585->VMAX - min_SHR,
> + IMX585_EXPOSURE_MIN, current_exposure);
> +
> + ret = imx585_write_reg_3byte(imx585, IMX585_REG_VMAX, imx585->VMAX);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_VMAX, ret);
> + break;
> + }
> + case V4L2_CID_HBLANK:
> + {
> + u64 pixel_rate;
> + u64 hmax;
> +
> + pixel_rate = (u64)mode->width * IMX585_PIXEL_RATE;
> + do_div(pixel_rate, mode->min_HMAX);
> + hmax = (u64)(mode->width + ctrl->val) * IMX585_PIXEL_RATE;
> + do_div(hmax, pixel_rate);
> + imx585->HMAX = hmax;
> +
> + dev_info(&client->dev, "V4L2_CID_HBLANK : %d\n", ctrl->val);
> + dev_info(&client->dev, "\tHMAX : %d\n", imx585->HMAX);
> +
> + ret = imx585_write_reg_2byte(imx585, IMX585_REG_HMAX, hmax);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_HMAX, ret);
> + break;
> + }
> + case V4L2_CID_HFLIP:
> + dev_info(&client->dev, "V4L2_CID_HFLIP : %d\n", ctrl->val);
> + ret = imx585_write_reg_1byte(imx585, IMX585_FLIP_WINMODEH, ctrl->val);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_FLIP_WINMODEH, ret);
> + break;
> + case V4L2_CID_VFLIP:
> + dev_info(&client->dev, "V4L2_CID_VFLIP : %d\n", ctrl->val);
> + ret = imx585_write_reg_1byte(imx585, IMX585_FLIP_WINMODEV, ctrl->val);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_FLIP_WINMODEV, ret);
> + break;
> + case V4L2_CID_BRIGHTNESS:
> + {
> + u16 blacklevel = ctrl->val;
> +
> + dev_info(&client->dev, "V4L2_CID_BRIGHTNESS : %d\n", ctrl->val);
> +
> + if (blacklevel > 4095)
> + blacklevel = 4095;
> + ret = imx585_write_reg_1byte(imx585, IMX585_REG_BLKLEVEL, blacklevel);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_BLKLEVEL, ret);
> + break;
> + }
> + case V4L2_CID_BAND_STOP_FILTER:
> + if (imx585->has_ircut) {
> + dev_info(&client->dev, "V4L2_CID_BAND_STOP_FILTER : %d\n", ctrl->val);
> + imx585_ircut_set(imx585, ctrl->val);
> + }
> + break;
> + case V4L2_CID_IMX585_HDR_DATASEL_TH:{
> + const u16 *th = (const u16 *)ctrl->p_new.p;
> +
> + ret = imx585_write_reg_2byte(imx585, IMX585_REG_EXP_TH_H, th[0]);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_EXP_TH_H, ret);
> + ret = imx585_write_reg_2byte(imx585, IMX585_REG_EXP_TH_L, th[1]);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_EXP_TH_L, ret);
> + dev_info(&client->dev, "V4L2_CID_IMX585_HDR_DATASEL_TH : %d, %d\n", th[0], th[1]);
> + break;
> + }
> + case V4L2_CID_IMX585_HDR_DATASEL_BK:
> + ret = imx585_write_reg_1byte(imx585, IMX585_REG_EXP_BK, ctrl->val);
> + dev_info(&client->dev, "V4L2_CID_IMX585_HDR_DATASEL_BK : %d\n", ctrl->val);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_EXP_BK, ret);
> + break;
> + case V4L2_CID_IMX585_HDR_GRAD_TH:{
> + const u32 *thr = (const u32 *)ctrl->p_new.p;
> +
> + ret = imx585_write_reg_3byte(imx585, IMX585_REG_CCMP1_EXP, thr[0]);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_CCMP1_EXP, ret);
> + ret = imx585_write_reg_3byte(imx585, IMX585_REG_CCMP2_EXP, thr[1]);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_CCMP2_EXP, ret);
> + dev_info(&client->dev, "V4L2_CID_IMX585_HDR_GRAD_TH : %d, %d\n", thr[0], thr[1]);
> + break;
> + }
> + case V4L2_CID_IMX585_HDR_GRAD_COMP_L:{
> + ret = imx585_write_reg_1byte(imx585, IMX585_REG_ACMP1_EXP, ctrl->val);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_ACMP1_EXP, ret);
> + dev_info(&client->dev, "V4L2_CID_IMX585_HDR_GRAD_COMP_L : %d\n", ctrl->val);
> + break;
> + }
> + case V4L2_CID_IMX585_HDR_GRAD_COMP_H:{
> + ret = imx585_write_reg_1byte(imx585, IMX585_REG_ACMP2_EXP, ctrl->val);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_ACMP2_EXP, ret);
> + dev_info(&client->dev, "V4L2_CID_IMX585_HDR_GRAD_COMP_H : %d\n", ctrl->val);
> + break;
> + }
> + case V4L2_CID_IMX585_HDR_GAIN:
> + ret = imx585_write_reg_1byte(imx585, IMX585_REG_EXP_GAIN, ctrl->val);
> + dev_info(&client->dev, "IMX585_REG_EXP_GAIN : %d\n", ctrl->val);
> + if (ret)
> + dev_err_ratelimited(&client->dev,
> + "Failed to write reg 0x%4.4x. error = %d\n",
> + IMX585_REG_EXP_GAIN, ret);
> + break;
> + case V4L2_CID_WIDE_DYNAMIC_RANGE:
> + /* Already handled above. */
> + break;
> + default:
> + dev_info(&client->dev,
> + "ctrl(id:0x%x,val:0x%x) is not handled\n",
> + ctrl->id, ctrl->val);
> + break;
> + }
> +
> + pm_runtime_put(&client->dev);
> +
> + return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops imx585_ctrl_ops = {
> + .s_ctrl = imx585_set_ctrl,
> +};
> +
> +static const u16 hdr_thresh_def[2] = { 512, 1024 };
> +static const struct v4l2_ctrl_config imx585_cfg_datasel_th = {
> + .ops = &imx585_ctrl_ops,
> + .id = V4L2_CID_IMX585_HDR_DATASEL_TH,
> + .name = "HDR Data selection Threshold",
> + .type = V4L2_CTRL_TYPE_U16,
> + .min = 0,
> + .max = 0x0FFF,
> + .step = 1,
> + .def = 0,
> + .dims = { 2 },
> + .elem_size = sizeof(u16),
> +};
> +
> +static const struct v4l2_ctrl_config imx585_cfg_datasel_bk = {
> + .ops = &imx585_ctrl_ops,
> + .id = V4L2_CID_IMX585_HDR_DATASEL_BK,
> + .name = "HDR Data Blending Mode",
> + .type = V4L2_CTRL_TYPE_MENU,
> + .max = ARRAY_SIZE(hdr_data_blender_menu) - 1,
> + .menu_skip_mask = 0,
> + .def = 0,
> + .qmenu = hdr_data_blender_menu,
> +};
> +
> +static const u32 grad_thresh_def[2] = { 500, 11500 };
> +static const struct v4l2_ctrl_config imx585_cfg_grad_th = {
> + .ops = &imx585_ctrl_ops,
> + .id = V4L2_CID_IMX585_HDR_GRAD_TH,
> + .name = "Gradiant Compression Threshold (16bit)",
> + .type = V4L2_CTRL_TYPE_U32,
> + .min = 0,
> + .max = 0x1FFFF,
> + .step = 1,
> + .def = 0,
> + .dims = { 2 },
> + .elem_size = sizeof(u32),
> +};
> +
> +static const struct v4l2_ctrl_config imx585_cfg_grad_exp_l = {
> + .ops = &imx585_ctrl_ops,
> + .id = V4L2_CID_IMX585_HDR_GRAD_COMP_L,
> + .name = "Gradiant Compression Ratio Low",
> + .type = V4L2_CTRL_TYPE_MENU,
> + .min = 0,
> + .max = ARRAY_SIZE(grad_compression_slope_menu) - 1,
> + .menu_skip_mask = 0,
> + .def = 2,
> + .qmenu = grad_compression_slope_menu,
> +};
> +
> +static const struct v4l2_ctrl_config imx585_cfg_grad_exp_h = {
> + .ops = &imx585_ctrl_ops,
> + .id = V4L2_CID_IMX585_HDR_GRAD_COMP_H,
> + .name = "Gradiant Compression Ratio High",
> + .type = V4L2_CTRL_TYPE_MENU,
> + .min = 0,
> + .max = ARRAY_SIZE(grad_compression_slope_menu) - 1,
> + .menu_skip_mask = 0,
> + .def = 6,
> + .qmenu = grad_compression_slope_menu,
> +};
> +
> +static const struct v4l2_ctrl_config imx585_cfg_hdr_gain = {
> + .ops = &imx585_ctrl_ops,
> + .id = V4L2_CID_IMX585_HDR_GAIN,
> + .name = "HDR Gain Adder (dB)",
> + .type = V4L2_CTRL_TYPE_MENU,
> + .min = 0,
> + .max = ARRAY_SIZE(hdr_gain_adder_menu) - 1,
> + .menu_skip_mask = 0,
> + .def = 2,
> + .qmenu = hdr_gain_adder_menu,
> +};
> +
> +static const struct v4l2_ctrl_config imx585_cfg_hcg = {
> + .ops = &imx585_ctrl_ops,
> + .id = V4L2_CID_IMX585_HCG_GAIN,
> + .name = "HCG Enable",
> + .type = V4L2_CTRL_TYPE_BOOLEAN,
> + .min = 0,
> + .max = 1,
> + .step = 1,
> + .def = 0,
> +};
> +
> +static int imx585_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + struct imx585 *imx585 = to_imx585(sd);
> + unsigned int entries;
> + const u32 *tbl;
> +
> + if (code->pad >= NUM_PADS)
> + return -EINVAL;
> +
> + if (code->pad == IMAGE_PAD) {
> + if (imx585->mono) {
> + if (imx585->clear_HDR) {
> + if (code->index > 1)
> + return -EINVAL;
> + code->code = mono_codes[code->index];
> + return 0;
> + }
> + /* HDR off: expose Y12 only */
> + if (code->index)
> + return -EINVAL;
> +
> + code->code = MEDIA_BUS_FMT_Y12_1X12;
> + return 0;
> + }
> +
> + if (imx585->clear_HDR) {
> + tbl = codes_clearhdr; /* << 16bit + 12bit */
> + entries = ARRAY_SIZE(codes_clearhdr) / 4;
> + } else {
> + tbl = codes_normal; /* << ONLY 12bit */
> + entries = ARRAY_SIZE(codes_normal) / 4;
> + }
> +
> + if (code->index >= entries)
> + return -EINVAL;
> +
> + code->code = imx585_get_format_code(imx585, tbl[code->index * 4]);
> + return 0;
> + }
> + /* --- Metadata pad ------------------------------------------------- */
> + if (code->index)
> + return -EINVAL;
> +
> + code->code = MEDIA_BUS_FMT_SENSOR_DATA;
> + return 0;
> +}
> +
> +static int imx585_enum_frame_size(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + struct imx585 *imx585 = to_imx585(sd);
> +
> + if (fse->pad >= NUM_PADS)
> + return -EINVAL;
> +
> + if (fse->pad == IMAGE_PAD) {
> + const struct imx585_mode *mode_list;
> + unsigned int num_modes;
> +
> + get_mode_table(imx585, fse->code, &mode_list, &num_modes);
> +
> + if (fse->index >= num_modes)
> + return -EINVAL;
> +
> + if (fse->code != imx585_get_format_code(imx585, fse->code))
> + return -EINVAL;
> +
> + fse->min_width = mode_list[fse->index].width;
> + fse->max_width = fse->min_width;
> + fse->min_height = mode_list[fse->index].height;
> + fse->max_height = fse->min_height;
> + } else {
> + if (fse->code != MEDIA_BUS_FMT_SENSOR_DATA || fse->index > 0)
> + return -EINVAL;
> +
> + fse->min_width = IMX585_EMBEDDED_LINE_WIDTH;
> + fse->max_width = fse->min_width;
> + fse->min_height = IMX585_NUM_EMBEDDED_LINES;
> + fse->max_height = fse->min_height;
> + }
> +
> + return 0;
> +}
> +
> +static void imx585_reset_colorspace(const struct imx585_mode *mode, struct v4l2_mbus_framefmt *fmt)
> +{
> + fmt->colorspace = V4L2_COLORSPACE_RAW;
> + fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
> + fmt->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
> + fmt->colorspace,
> + fmt->ycbcr_enc);
> + fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
> +}
> +
> +static void imx585_update_image_pad_format(struct imx585 *imx585,
> + const struct imx585_mode *mode,
> + struct v4l2_subdev_format *fmt)
> +{
> + fmt->format.width = mode->width;
> + fmt->format.height = mode->height;
> + fmt->format.field = V4L2_FIELD_NONE;
> + imx585_reset_colorspace(mode, &fmt->format);
> +}
> +
> +static void imx585_update_metadata_pad_format(struct v4l2_subdev_format *fmt)
> +{
> + fmt->format.width = IMX585_EMBEDDED_LINE_WIDTH;
> + fmt->format.height = IMX585_NUM_EMBEDDED_LINES;
> + fmt->format.code = MEDIA_BUS_FMT_SENSOR_DATA;
> + fmt->format.field = V4L2_FIELD_NONE;
> +}
> +
> +static int imx585_get_pad_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct imx585 *imx585 = to_imx585(sd);
> +
> + if (fmt->pad >= NUM_PADS)
> + return -EINVAL;
> +
> + mutex_lock(&imx585->mutex);
> +
> + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> + struct v4l2_mbus_framefmt *try_fmt =
> + v4l2_subdev_state_get_format(sd_state, fmt->pad);
> + /* update the code which could change due to vflip or hflip: */
> + try_fmt->code = fmt->pad == IMAGE_PAD ?
> + imx585_get_format_code(imx585, try_fmt->code) :
> + MEDIA_BUS_FMT_SENSOR_DATA;
> + fmt->format = *try_fmt;
> + } else {
> + if (fmt->pad == IMAGE_PAD) {
> + imx585_update_image_pad_format(imx585, imx585->mode, fmt);
> + fmt->format.code =
> + imx585_get_format_code(imx585, imx585->fmt_code);
> + } else {
> + imx585_update_metadata_pad_format(fmt);
> + }
> + }
> +
> + mutex_unlock(&imx585->mutex);
> + return 0;
> +}
> +
> +static int imx585_set_pad_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct v4l2_mbus_framefmt *framefmt;
> + const struct imx585_mode *mode;
> + struct imx585 *imx585 = to_imx585(sd);
> +
> + if (fmt->pad >= NUM_PADS)
> + return -EINVAL;
> +
> + mutex_lock(&imx585->mutex);
> +
> + if (fmt->pad == IMAGE_PAD) {
> + const struct imx585_mode *mode_list;
> + unsigned int num_modes;
> +
> + /* Bayer order varies with flips */
> + fmt->format.code = imx585_get_format_code(imx585, fmt->format.code);
> + get_mode_table(imx585, fmt->format.code, &mode_list, &num_modes);
> + mode = v4l2_find_nearest_size(mode_list,
> + num_modes,
> + width, height,
> + fmt->format.width,
> + fmt->format.height);
> + imx585_update_image_pad_format(imx585, mode, fmt);
> + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> + framefmt = v4l2_subdev_state_get_format(sd_state, fmt->pad);
> + *framefmt = fmt->format;
> + } else if (imx585->mode != mode ||
> + imx585->fmt_code != fmt->format.code) {
> + imx585->mode = mode;
> + imx585->fmt_code = fmt->format.code;
> + imx585_set_framing_limits(imx585);
> + }
> + } else {
> + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> + framefmt = v4l2_subdev_state_get_format(sd_state, fmt->pad);
> + *framefmt = fmt->format;
> + } else {
> + /* Only one embedded data mode is supported */
> + imx585_update_metadata_pad_format(fmt);
> + }
> + }
> +
> + mutex_unlock(&imx585->mutex);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_rect *
> +__imx585_get_pad_crop(struct imx585 *imx585,
> + struct v4l2_subdev_state *sd_state,
> + unsigned int pad, enum v4l2_subdev_format_whence which)
> +{
> + switch (which) {
> + case V4L2_SUBDEV_FORMAT_TRY:
> + return v4l2_subdev_state_get_crop(sd_state, IMAGE_PAD);
> + case V4L2_SUBDEV_FORMAT_ACTIVE:
> + return &imx585->mode->crop;
> + }
> +
> + return NULL;
> +}
> +
> +/* Start streaming */
> +static int imx585_start_streaming(struct imx585 *imx585)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + const struct IMX585_reg_list *reg_list;
> + int ret;
> +
> + if (!imx585->common_regs_written) {
> + ret = imx585_write_regs(imx585, common_regs, ARRAY_SIZE(common_regs));
> + if (ret) {
> + dev_err(&client->dev, "%s failed to set common settings\n", __func__);
> + return ret;
> + }
> +
> + imx585_write_reg_1byte(imx585, IMX585_INCK_SEL, imx585->inck_sel_val);
> + imx585_write_reg_2byte(imx585, IMX585_REG_BLKLEVEL, IMX585_BLKLEVEL_DEFAULT);
> + imx585_write_reg_1byte(imx585, IMX585_DATARATE_SEL,
> + link_freqs_reg_value[imx585->link_freq_idx]);
> +
> + if (imx585->lane_count == 2)
> + imx585_write_reg_1byte(imx585, IMX585_LANEMODE, 0x01);
> + else
> + imx585_write_reg_1byte(imx585, IMX585_LANEMODE, 0x03);
> +
> + if (imx585->mono)
> + imx585_write_reg_1byte(imx585, IMX585_BIN_MODE, 0x01);
> + else
> + imx585_write_reg_1byte(imx585, IMX585_BIN_MODE, 0x00);
> +
> + if (imx585->sync_mode == 1) { //External Sync Leader Mode
> + dev_info(&client->dev, "External Sync Leader Mode, enable XVS input\n");
> + imx585_write_reg_1byte(imx585, IMX585_REG_EXTMODE, 0x01);
> + // Enable XHS output, but XVS is input
> + imx585_write_reg_1byte(imx585, IMX585_REG_XXS_DRV, 0x03);
> + // Disable XVS OUT
> + imx585_write_reg_1byte(imx585, IMX585_REG_XXS_OUTSEL, 0x08);
> + } else if (imx585->sync_mode == 0) { //Internal Sync Leader Mode
> + dev_info(&client->dev, "Internal Sync Leader Mode, enable output\n");
> + imx585_write_reg_1byte(imx585, IMX585_REG_EXTMODE, 0x00);
> + // Enable XHS and XVS output
> + imx585_write_reg_1byte(imx585, IMX585_REG_XXS_DRV, 0x00);
> + imx585_write_reg_1byte(imx585, IMX585_REG_XXS_OUTSEL, 0x0A);
> + } else {
> + dev_info(&client->dev, "Follower Mode, enable XVS/XHS input\n");
> + //For follower mode, switch both of them to input
> + imx585_write_reg_1byte(imx585, IMX585_REG_XXS_DRV, 0x0F);
> + imx585_write_reg_1byte(imx585, IMX585_REG_XXS_OUTSEL, 0x00);
> + }
> + imx585->common_regs_written = true;
> + dev_info(&client->dev, "common_regs_written\n");
> + }
> +
> + /* Apply default values of current mode */
> + reg_list = &imx585->mode->reg_list;
> + ret = imx585_write_regs(imx585, reg_list->regs, reg_list->num_of_regs);
> + if (ret) {
> + dev_err(&client->dev, "%s failed to set mode\n", __func__);
> + return ret;
> + }
> +
> + if (imx585->clear_HDR) {
> + ret = imx585_write_regs(imx585, common_clearHDR_mode,
> + ARRAY_SIZE(common_clearHDR_mode));
> + if (ret) {
> + dev_err(&client->dev, "%s failed to set ClearHDR settings\n", __func__);
> + return ret;
> + }
> + //16bit mode is linear, 12bit mode we need to enable gradation compression
> + switch (imx585->fmt_code) {
> + /* 16-bit */
> + case MEDIA_BUS_FMT_SRGGB16_1X16:
> + case MEDIA_BUS_FMT_SGRBG16_1X16:
> + case MEDIA_BUS_FMT_SGBRG16_1X16:
> + case MEDIA_BUS_FMT_SBGGR16_1X16:
> + case MEDIA_BUS_FMT_Y16_1X16:
> + imx585_write_reg_1byte(imx585, IMX595_REG_CCMP_EN, 0);
> + imx585_write_reg_1byte(imx585, 0x3023, 0x03); // MDBIT 16-bit
> + dev_info(&client->dev, "16bit HDR written\n");
> + break;
> + /* 12-bit */
> + case MEDIA_BUS_FMT_SRGGB12_1X12:
> + case MEDIA_BUS_FMT_SGRBG12_1X12:
> + case MEDIA_BUS_FMT_SGBRG12_1X12:
> + case MEDIA_BUS_FMT_SBGGR12_1X12:
> + case MEDIA_BUS_FMT_Y12_1X12:
> + imx585_write_reg_1byte(imx585, IMX595_REG_CCMP_EN, 1);
> + dev_info(&client->dev, "12bit HDR written\n");
> + break;
> + default:
> + break;
> + }
> + dev_info(&client->dev, "ClearHDR_regs_written\n");
> +
> + } else {
> + ret = imx585_write_regs(imx585, common_normal_mode, ARRAY_SIZE(common_normal_mode));
> + if (ret) {
> + dev_err(&client->dev, "%s failed to set Normal settings\n", __func__);
> + return ret;
> + }
> + dev_info(&client->dev, "normal_regs_written\n");
> + }
> +
> + /* Disable digital clamp */
> + imx585_write_reg_1byte(imx585, IMX585_REG_DIGITAL_CLAMP, 0);
> +
> + /* Apply customized values from user */
> + ret = __v4l2_ctrl_handler_setup(imx585->sd.ctrl_handler);
> + if (ret) {
> + dev_err(&client->dev, "%s failed to apply user values\n", __func__);
> + return ret;
> + }
> +
> + if (imx585->sync_mode <= 1) {
> + dev_info(&client->dev, "imx585 Leader mode enabled\n");
> + imx585_write_reg_1byte(imx585, IMX585_REG_XMSTA, 0x00);
> + }
> +
> + /* Set stream on register */
> + ret = imx585_write_reg_1byte(imx585, IMX585_REG_MODE_SELECT, IMX585_MODE_STREAMING);
> +
> + dev_info(&client->dev, "Start Streaming\n");
> + usleep_range(IMX585_STREAM_DELAY_US, IMX585_STREAM_DELAY_US + IMX585_STREAM_DELAY_RANGE_US);
> + return ret;
> +}
> +
> +/* Stop streaming */
> +static void imx585_stop_streaming(struct imx585 *imx585)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + int ret;
> +
> + dev_info(&client->dev, "Stop Streaming\n");
> +
> + /* set stream off register */
> + ret = imx585_write_reg_1byte(imx585, IMX585_REG_MODE_SELECT, IMX585_MODE_STANDBY);
> + if (ret)
> + dev_err(&client->dev, "%s failed to stop stream\n", __func__);
> +}
> +
> +static int imx585_set_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct imx585 *imx585 = to_imx585(sd);
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> + int ret = 0;
> +
> + mutex_lock(&imx585->mutex);
> + if (imx585->streaming == enable) {
> + mutex_unlock(&imx585->mutex);
> + return 0;
> + }
> +
> + if (enable) {
> + ret = pm_runtime_get_sync(&client->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(&client->dev);
> + goto err_unlock;
> + }
> +
> + /*
> + * Apply default & customized values
> + * and then start streaming.
> + */
> + ret = imx585_start_streaming(imx585);
> + if (ret)
> + goto err_rpm_put;
> + } else {
> + imx585_stop_streaming(imx585);
> + pm_runtime_put(&client->dev);
> + }
> +
> + imx585->streaming = enable;
> +
> + /* vflip/hflip and hdr mode cannot change during streaming */
> + __v4l2_ctrl_grab(imx585->vflip, enable);
> + __v4l2_ctrl_grab(imx585->hflip, enable);
> + __v4l2_ctrl_grab(imx585->hdr_mode, enable);
> +
> + mutex_unlock(&imx585->mutex);
> +
> + return ret;
> +
> +err_rpm_put:
> + pm_runtime_put(&client->dev);
> +err_unlock:
> + mutex_unlock(&imx585->mutex);
> +
> + return ret;
> +}
> +
> +/* Power/clock management functions */
> +static int imx585_power_on(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx585 *imx585 = to_imx585(sd);
> + int ret;
> +
> + ret = regulator_bulk_enable(imx585_NUM_SUPPLIES,
> + imx585->supplies);
> + if (ret) {
> + dev_err(&client->dev, "%s: failed to enable regulators\n",
> + __func__);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(imx585->xclk);
> + if (ret) {
> + dev_err(&client->dev, "%s: failed to enable clock\n",
> + __func__);
> + goto reg_off;
> + }
> +
> + gpiod_set_value_cansleep(imx585->reset_gpio, 1);
> + usleep_range(IMX585_XCLR_MIN_DELAY_US,
> + IMX585_XCLR_MIN_DELAY_US + IMX585_XCLR_DELAY_RANGE_US);
> +
> + return 0;
> +
> +reg_off:
> + regulator_bulk_disable(imx585_NUM_SUPPLIES, imx585->supplies);
> + return ret;
> +}
> +
> +static int imx585_power_off(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx585 *imx585 = to_imx585(sd);
> +
> + gpiod_set_value_cansleep(imx585->reset_gpio, 0);
> + regulator_bulk_disable(imx585_NUM_SUPPLIES, imx585->supplies);
> + clk_disable_unprepare(imx585->xclk);
> +
> + /* Force reprogramming of the common registers when powered up again. */
> + imx585->common_regs_written = false;
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx585_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx585 *imx585 = to_imx585(sd);
> +
> + if (imx585->streaming)
> + imx585_stop_streaming(imx585);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx585_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx585 *imx585 = to_imx585(sd);
> + int ret;
> +
> + if (imx585->streaming) {
> + ret = imx585_start_streaming(imx585);
> + if (ret)
> + goto error;
> + }
> +
> + return 0;
> +
> +error:
> + imx585_stop_streaming(imx585);
> + imx585->streaming = 0;
> + return ret;
> +}
> +
> +static int imx585_get_regulators(struct imx585 *imx585)
Please move this function and the next just before the probe function,
to keep all probe code grouped together.
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + unsigned int i;
> +
> + for (i = 0; i < imx585_NUM_SUPPLIES; i++)
> + imx585->supplies[i].supply = imx585_supply_name[i];
> +
> + return devm_regulator_bulk_get(&client->dev,
> + imx585_NUM_SUPPLIES,
> + imx585->supplies);
> +}
> +
> +/* Verify chip ID */
> +static int imx585_check_module_exists(struct imx585 *imx585)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + int ret;
> + u32 val;
> +
> + /* We don't actually have a CHIP ID register so we try to read from BLKLEVEL instead*/
> + ret = imx585_read_reg(imx585, IMX585_REG_BLKLEVEL,
> + 1, &val);
> + if (ret) {
> + dev_err(&client->dev, "failed to read chip reg, with error %d\n", ret);
> + return ret;
> + }
> +
> + dev_info(&client->dev, "Reg read success, Device found\n");
> +
> + return 0;
> +}
> +
> +static int imx585_get_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_selection *sel)
> +{
> + switch (sel->target) {
> + case V4L2_SEL_TGT_CROP: {
> + struct imx585 *imx585 = to_imx585(sd);
> +
> + mutex_lock(&imx585->mutex);
> + sel->r = *__imx585_get_pad_crop(imx585, sd_state, sel->pad, sel->which);
> + mutex_unlock(&imx585->mutex);
> +
> + return 0;
> + }
> +
> + case V4L2_SEL_TGT_NATIVE_SIZE:
> + sel->r.left = 0;
> + sel->r.top = 0;
> + sel->r.width = IMX585_NATIVE_WIDTH;
> + sel->r.height = IMX585_NATIVE_HEIGHT;
> + return 0;
> +
> + case V4L2_SEL_TGT_CROP_DEFAULT:
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + sel->r.left = IMX585_PIXEL_ARRAY_LEFT;
> + sel->r.top = IMX585_PIXEL_ARRAY_TOP;
> + sel->r.width = IMX585_PIXEL_ARRAY_WIDTH;
> + sel->r.height = IMX585_PIXEL_ARRAY_HEIGHT;
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct v4l2_subdev_core_ops imx585_core_ops = {
> + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> +};
> +
> +static const struct v4l2_subdev_video_ops imx585_video_ops = {
> + .s_stream = imx585_set_stream,
Please implement the .enable_streams() and .disable_streams() pad
ops, and use v4l2_subdev_s_stream_helper here.
> +};
> +
> +static const struct v4l2_subdev_pad_ops imx585_pad_ops = {
> + .enum_mbus_code = imx585_enum_mbus_code,
> + .get_fmt = imx585_get_pad_format,
> + .set_fmt = imx585_set_pad_format,
> + .get_selection = imx585_get_selection,
> + .enum_frame_size = imx585_enum_frame_size,
> +};
> +
> +static const struct v4l2_subdev_ops imx585_subdev_ops = {
> + .core = &imx585_core_ops,
> + .video = &imx585_video_ops,
> + .pad = &imx585_pad_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops imx585_internal_ops = {
> + .open = imx585_open,
> +};
> +
> +/* Initialize control handlers */
> +static int imx585_init_controls(struct imx585 *imx585)
Please the control-related functions just below imx585_cfg_hcg, to keep
the data and functions grouped together.
> +{
> + struct v4l2_ctrl_handler *ctrl_hdlr;
> + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> + struct v4l2_fwnode_device_properties props;
> + int ret;
> +
> + ctrl_hdlr = &imx585->ctrl_handler;
> + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 32);
> + if (ret)
> + return ret;
> +
> + mutex_init(&imx585->mutex);
> + ctrl_hdlr->lock = &imx585->mutex;
> +
> + /*
> + * Create the controls here, but mode specific limits are setup
> + * in the imx585_set_framing_limits() call below.
> + */
> + /* By default, PIXEL_RATE is read only */
> + imx585->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
> + V4L2_CID_PIXEL_RATE,
> + 0xffff,
> + 0xffff, 1,
> + 0xffff);
> +
> + /* LINK_FREQ is also read only */
> + imx585->link_freq =
> + v4l2_ctrl_new_int_menu(ctrl_hdlr, &imx585_ctrl_ops,
> + V4L2_CID_LINK_FREQ, 0, 0,
> + &link_freqs[imx585->link_freq_idx]);
> + if (imx585->link_freq)
> + imx585->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + imx585->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
> + V4L2_CID_VBLANK, 0, 0xfffff, 1, 0);
> + imx585->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
> + V4L2_CID_HBLANK, 0, 0xffff, 1, 0);
> + imx585->blacklevel = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
> + V4L2_CID_BRIGHTNESS, 0, 0xffff, 1,
> + IMX585_BLKLEVEL_DEFAULT);
> +
> + imx585->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
> + V4L2_CID_EXPOSURE,
> + IMX585_EXPOSURE_MIN,
> + IMX585_EXPOSURE_MAX,
> + IMX585_EXPOSURE_STEP,
> + IMX585_EXPOSURE_DEFAULT);
> +
> + imx585->gain = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops, V4L2_CID_ANALOGUE_GAIN,
> + IMX585_ANA_GAIN_MIN_NORMAL, IMX585_ANA_GAIN_MAX_NORMAL,
> + IMX585_ANA_GAIN_STEP, IMX585_ANA_GAIN_DEFAULT);
> +
> + imx585->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops, V4L2_CID_HFLIP, 0, 1, 1, 0);
> + imx585->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0);
> +
> + if (imx585->has_ircut) {
> + imx585->ircut_ctrl =
> + v4l2_ctrl_new_std(&imx585->ctrl_handler, &imx585_ctrl_ops,
> + V4L2_CID_BAND_STOP_FILTER,
> + 0, 1, 1, 1);
> + }
> +
> + imx585->hdr_mode = v4l2_ctrl_new_std(ctrl_hdlr, &imx585_ctrl_ops,
> + V4L2_CID_WIDE_DYNAMIC_RANGE,
> + 0, 1, 1, 0);
> + imx585->datasel_th_ctrl = v4l2_ctrl_new_custom(ctrl_hdlr,
> + &imx585_cfg_datasel_th, NULL);
> + imx585->datasel_bk_ctrl = v4l2_ctrl_new_custom(ctrl_hdlr,
> + &imx585_cfg_datasel_bk, NULL);
> + imx585->gdc_th_ctrl = v4l2_ctrl_new_custom(ctrl_hdlr,
> + &imx585_cfg_grad_th, NULL);
> + imx585->gdc_exp_ctrl_l = v4l2_ctrl_new_custom(ctrl_hdlr,
> + &imx585_cfg_grad_exp_l, NULL);
> + imx585->gdc_exp_ctrl_h = v4l2_ctrl_new_custom(ctrl_hdlr,
> + &imx585_cfg_grad_exp_h, NULL);
> + imx585->hdr_gain_ctrl = v4l2_ctrl_new_custom(ctrl_hdlr,
> + &imx585_cfg_hdr_gain, NULL);
> + imx585->hcg_ctrl = v4l2_ctrl_new_custom(ctrl_hdlr,
> + &imx585_cfg_hcg, NULL);
> +
> + v4l2_ctrl_activate(imx585->datasel_th_ctrl, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->datasel_bk_ctrl, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->gdc_th_ctrl, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->gdc_exp_ctrl_l, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->gdc_exp_ctrl_h, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->hdr_gain_ctrl, imx585->clear_HDR);
> + v4l2_ctrl_activate(imx585->hcg_ctrl, !imx585->clear_HDR);
> +
> + if (ctrl_hdlr->error) {
> + ret = ctrl_hdlr->error;
> + dev_err(&client->dev, "%s control init failed (%d)\n",
> + __func__, ret);
> + goto error;
> + }
> +
> + ret = v4l2_fwnode_device_parse(&client->dev, &props);
> + if (ret)
> + goto error;
> +
> + ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &imx585_ctrl_ops, &props);
> + if (ret)
> + goto error;
> +
> + memcpy(imx585->datasel_th_ctrl->p_cur.p, hdr_thresh_def, sizeof(hdr_thresh_def));
> + memcpy(imx585->datasel_th_ctrl->p_new.p, hdr_thresh_def, sizeof(hdr_thresh_def));
> + memcpy(imx585->gdc_th_ctrl->p_cur.p, grad_thresh_def, sizeof(grad_thresh_def));
> + memcpy(imx585->gdc_th_ctrl->p_new.p, grad_thresh_def, sizeof(grad_thresh_def));
> +
> + imx585->hdr_mode->flags |= V4L2_CTRL_FLAG_UPDATE;
> + imx585->hdr_mode->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> +
> + imx585->sd.ctrl_handler = ctrl_hdlr;
> +
> + /* Setup exposure and frame/line length limits. */
> + imx585_set_framing_limits(imx585);
> +
> + return 0;
> +
> +error:
> + v4l2_ctrl_handler_free(ctrl_hdlr);
> + mutex_destroy(&imx585->mutex);
> +
> + return ret;
> +}
> +
> +static void imx585_free_controls(struct imx585 *imx585)
> +{
> + v4l2_ctrl_handler_free(imx585->sd.ctrl_handler);
> + mutex_destroy(&imx585->mutex);
> +}
> +
> +static const struct of_device_id imx585_dt_ids[] = {
> + { .compatible = "sony,imx585"},
> + { /* sentinel */ }
> +};
> +
> +static int imx585_check_hwcfg(struct device *dev, struct imx585 *imx585)
> +{
> + struct fwnode_handle *endpoint;
struct fwnode_handle *endpoint __free(fwnode_handle_put) = NULL;
and rop the fwnode_handle_put() call below. That will simplify error
handling.
> + struct v4l2_fwnode_endpoint ep_cfg = {
> + .bus_type = V4L2_MBUS_CSI2_DPHY
> + };
> + int ret = -EINVAL;
> + int i;
> +
> + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
> + if (!endpoint) {
> + dev_err(dev, "endpoint node not found\n");
> + return -EINVAL;
> + }
> +
> + if (v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep_cfg)) {
> + dev_err(dev, "could not parse endpoint\n");
> + goto error_out;
> + }
> +
> + /* Check the number of MIPI CSI2 data lanes */
> + if (ep_cfg.bus.mipi_csi2.num_data_lanes != 2 && ep_cfg.bus.mipi_csi2.num_data_lanes != 4) {
> + dev_err(dev, "only 2 or 4 data lanes are currently supported\n");
> + goto error_out;
> + }
> + imx585->lane_count = ep_cfg.bus.mipi_csi2.num_data_lanes;
> + dev_info(dev, "Data lanes: %d\n", imx585->lane_count);
> +
> + /* Check the link frequency set in device tree */
> + if (!ep_cfg.nr_of_link_frequencies) {
> + dev_err(dev, "link-frequency property not found in DT\n");
> + goto error_out;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(link_freqs); i++) {
> + if (link_freqs[i] == ep_cfg.link_frequencies[0]) {
> + imx585->link_freq_idx = i;
> + break;
> + }
> + }
> +
> + if (i == ARRAY_SIZE(link_freqs)) {
> + dev_err(dev, "Link frequency not supported: %lld\n",
> + ep_cfg.link_frequencies[0]);
> + ret = -EINVAL;
> + goto error_out;
> + }
> +
> + dev_info(dev, "Link Speed: %lld Mhz\n", ep_cfg.link_frequencies[0]);
> +
> + ret = 0;
> +
> +error_out:
> + v4l2_fwnode_endpoint_free(&ep_cfg);
> + fwnode_handle_put(endpoint);
> +
> + return ret;
> +}
> +
> +static int imx585_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct device_node *np;
> + struct imx585 *imx585;
> + const struct of_device_id *match;
> + int ret, i;
> + u32 sync_mode;
> +
> + imx585 = devm_kzalloc(&client->dev, sizeof(*imx585), GFP_KERNEL);
> + if (!imx585)
> + return -ENOMEM;
> +
> + v4l2_i2c_subdev_init(&imx585->sd, client, &imx585_subdev_ops);
> +
> + match = of_match_device(imx585_dt_ids, dev);
> + if (!match)
> + return -ENODEV;
This doesn't seem needed.
> +
> + dev_info(dev, "Reading dtoverlay config:\n");
> + imx585->mono = of_property_read_bool(dev->of_node, "mono-mode");
Mono sensors should be detected based on the compatible string, not with
a separate property.
> + if (imx585->mono)
> + dev_info(dev, "Mono Mode Selected, make sure you have the correct sensor variant\n");
> +
> + imx585->clear_HDR = of_property_read_bool(dev->of_node, "clearHDR-mode");
This doesn't seem to belong to DT, as it's a runtime option.
> + dev_info(dev, "ClearHDR: %d\n", imx585->clear_HDR);
> +
> + imx585->sync_mode = 0;
> + ret = of_property_read_u32(dev->of_node, "sync-mode", &sync_mode);
> + if (!ret) {
> + if (sync_mode > 2) {
> + dev_warn(dev, "sync-mode out of range, using 0\n");
> + sync_mode = 0;
> + }
> + imx585->sync_mode = sync_mode;
> + } else if (ret != -EINVAL) { /* property present but bad */
> + dev_err(dev, "sync-mode malformed (%pe)\n", ERR_PTR(ret));
> + return ret;
> + }
> + dev_info(dev, "Sync Mode: %s\n", sync_mode_menu[imx585->sync_mode]);
> +
> + /* Check the hardware configuration in device tree */
> + if (imx585_check_hwcfg(dev, imx585))
> + return -EINVAL;
> +
> + /* Get system clock (xclk) */
> + imx585->xclk = devm_clk_get(dev, NULL);
> + if (IS_ERR(imx585->xclk)) {
> + dev_err(dev, "failed to get xclk\n");
> + return PTR_ERR(imx585->xclk);
> + }
> +
> + imx585->xclk_freq = clk_get_rate(imx585->xclk);
> +
> + for (i = 0; i < ARRAY_SIZE(imx585_inck_table); ++i) {
> + if (imx585_inck_table[i].xclk_hz == imx585->xclk_freq) {
> + imx585->inck_sel_val = imx585_inck_table[i].inck_sel;
> + break;
> + }
> + }
> +
> + if (i == ARRAY_SIZE(imx585_inck_table)) {
> + dev_err(dev, "unsupported XCLK rate %u Hz\n",
> + imx585->xclk_freq);
> + return -EINVAL;
> + }
> +
> + dev_info(dev, "XCLK %u Hz → INCK_SEL 0x%02x\n",
> + imx585->xclk_freq, imx585->inck_sel_val);
> +
> + ret = imx585_get_regulators(imx585);
> + if (ret) {
> + dev_err(dev, "failed to get regulators\n");
> + return ret;
> + }
> +
> + /* Request optional enable pin */
> + imx585->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> + GPIOD_OUT_HIGH);
> +
> + /*
> + * The sensor must be powered for imx585_check_module_exists()
> + * to be able to read register
> + */
> + ret = imx585_power_on(dev);
> + if (ret)
> + return ret;
> +
> + ret = imx585_check_module_exists(imx585);
> + if (ret)
> + goto error_power_off;
> +
> + imx585->has_ircut = false;
> + imx585->ircut_client = NULL;
> +
> + if (of_property_read_bool(dev->of_node, "ircut-mode")) {
> + np = of_parse_phandle(dev->of_node, "ircut-controller", 0);
> + if (np) {
> + imx585->ircut_client = of_find_i2c_device_by_node(np);
> + of_node_put(np);
> + ret = imx585_ircut_write(imx585, 0x01);
> + if (!ret) {
> + imx585->has_ircut = true;
> + dev_info(dev, "IR-cut controller present at 0x%02x\n",
> + imx585->ircut_client->addr);
> + } else {
> + dev_info(dev,
> + "Can't communication with IR-cut control, dropping\n");
> + }
> + }
> + } else {
> + dev_info(dev, "No IR-cut controller\n");
> + }
> +
> + /* Initialize default format */
> + imx585_set_default_format(imx585);
> +
> + /* Enable runtime PM and turn off the device */
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> + pm_runtime_idle(dev);
It would be best to use PM runtime autosuspend. See the imx296 driver
for an example.
> +
> + /* This needs the pm runtime to be registered. */
> + ret = imx585_init_controls(imx585);
> + if (ret)
> + goto error_pm_runtime;
> +
> + /* Initialize subdev */
> + imx585->sd.internal_ops = &imx585_internal_ops;
> + imx585->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> + V4L2_SUBDEV_FL_HAS_EVENTS;
> + imx585->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> +
> + /* Initialize source pads */
> + imx585->pad[IMAGE_PAD].flags = MEDIA_PAD_FL_SOURCE;
> + imx585->pad[METADATA_PAD].flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&imx585->sd.entity, NUM_PADS, imx585->pad);
> + if (ret) {
> + dev_err(dev, "failed to init entity pads: %d\n", ret);
> + goto error_handler_free;
> + }
> +
> + ret = v4l2_async_register_subdev_sensor(&imx585->sd);
> + if (ret < 0) {
> + dev_err(dev, "failed to register sensor sub-device: %d\n", ret);
> + goto error_media_entity;
> + }
> +
> + return 0;
> +
> +error_media_entity:
> + media_entity_cleanup(&imx585->sd.entity);
> +
> +error_handler_free:
> + imx585_free_controls(imx585);
> +
> +error_pm_runtime:
> + pm_runtime_disable(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> +
> +error_power_off:
> + imx585_power_off(&client->dev);
> +
> + return ret;
> +}
> +
> +static void imx585_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx585 *imx585 = to_imx585(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
> + imx585_free_controls(imx585);
> +
> + pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev))
> + imx585_power_off(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> +}
> +
> +MODULE_DEVICE_TABLE(of, imx585_dt_ids);
This should go just below imx585_dt_ids. I would move imx585_dt_ids just
above the MODULE_DEVICE_TABLE macro.
> +
> +static const struct dev_pm_ops imx585_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(imx585_suspend, imx585_resume)
> + SET_RUNTIME_PM_OPS(imx585_power_off, imx585_power_on, NULL)
> +};
> +
> +static struct i2c_driver imx585_i2c_driver = {
> + .driver = {
> + .name = "imx585",
> + .of_match_table = imx585_dt_ids,
> + .pm = &imx585_pm_ops,
> + },
> + .probe = imx585_probe,
> + .remove = imx585_remove,
> +};
> +
> +module_i2c_driver(imx585_i2c_driver);
> +
> +MODULE_AUTHOR("Will Whang <will@willwhang.com>");
> +MODULE_AUTHOR("Tetsuya NOMURA <tetsuya.nomura@soho-enterprise.com>");
> +MODULE_DESCRIPTION("Sony imx585 sensor driver");
s/imx585/IMX585/
> +MODULE_LICENSE("GPL");
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 3/4] media: i2c: imx585: Add Sony IMX585 image-sensor driver
2025-07-03 17:51 ` Laurent Pinchart
@ 2025-07-03 17:54 ` Laurent Pinchart
[not found] ` <CAFoNnrx-YpQwY6_908x=8LK1uwWw0y5zKxsv+aTsW1fxX554vg@mail.gmail.com>
1 sibling, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2025-07-03 17:54 UTC (permalink / raw)
To: Will Whang
Cc: Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
A few other comments.
On Thu, Jul 03, 2025 at 08:51:23PM +0300, Laurent Pinchart wrote:
> Hi Will,
>
> Thank you for the patch.
>
> Here's a first review, focussing on API usage and coding style.
>
> On Wed, Jul 02, 2025 at 07:38:35AM +0100, Will Whang wrote:
> > Implements support for:
> > * 4-lane / 2-lane CSI-2
> > * 12-bit linear, 12-bit HDR-GC and 16-bit Clear-HDR modes
> > * Mono variant switch, HCG, custom HDR controls
> > * Tested on Raspberry Pi 4/5 with 24 MHz XCLK.
> >
> > Signed-off-by: Will Whang <will@willwhang.com>
> > ---
> > drivers/media/i2c/Kconfig | 9 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/imx585.c | 2466 ++++++++++++++++++++++++++++++++++++
> > 3 files changed, 2476 insertions(+)
> > create mode 100644 drivers/media/i2c/imx585.c
[snip]
> > diff --git a/drivers/media/i2c/imx585.c b/drivers/media/i2c/imx585.c
> > new file mode 100644
> > index 000000000..2c4212290
> > --- /dev/null
> > +++ b/drivers/media/i2c/imx585.c
> > @@ -0,0 +1,2466 @@
[snip]
> > +static int imx585_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct device_node *np;
> > + struct imx585 *imx585;
> > + const struct of_device_id *match;
> > + int ret, i;
> > + u32 sync_mode;
> > +
> > + imx585 = devm_kzalloc(&client->dev, sizeof(*imx585), GFP_KERNEL);
> > + if (!imx585)
> > + return -ENOMEM;
> > +
> > + v4l2_i2c_subdev_init(&imx585->sd, client, &imx585_subdev_ops);
> > +
> > + match = of_match_device(imx585_dt_ids, dev);
> > + if (!match)
> > + return -ENODEV;
>
> This doesn't seem needed.
>
> > +
> > + dev_info(dev, "Reading dtoverlay config:\n");
> > + imx585->mono = of_property_read_bool(dev->of_node, "mono-mode");
This is not defined in the DT bindings.
> Mono sensors should be detected based on the compatible string, not with
> a separate property.
>
> > + if (imx585->mono)
> > + dev_info(dev, "Mono Mode Selected, make sure you have the correct sensor variant\n");
> > +
> > + imx585->clear_HDR = of_property_read_bool(dev->of_node, "clearHDR-mode");
Neither is this.
>
> This doesn't seem to belong to DT, as it's a runtime option.
>
> > + dev_info(dev, "ClearHDR: %d\n", imx585->clear_HDR);
> > +
> > + imx585->sync_mode = 0;
> > + ret = of_property_read_u32(dev->of_node, "sync-mode", &sync_mode);
In the DT binding, the sync-mode is currently specified in the endpoint,
not in the device node. I've recommended in my review of 1/4 to move it
to the device node, so this will become correct.
> > + if (!ret) {
> > + if (sync_mode > 2) {
> > + dev_warn(dev, "sync-mode out of range, using 0\n");
> > + sync_mode = 0;
> > + }
> > + imx585->sync_mode = sync_mode;
> > + } else if (ret != -EINVAL) { /* property present but bad */
> > + dev_err(dev, "sync-mode malformed (%pe)\n", ERR_PTR(ret));
> > + return ret;
> > + }
> > + dev_info(dev, "Sync Mode: %s\n", sync_mode_menu[imx585->sync_mode]);
> > +
> > + /* Check the hardware configuration in device tree */
> > + if (imx585_check_hwcfg(dev, imx585))
> > + return -EINVAL;
> > +
> > + /* Get system clock (xclk) */
> > + imx585->xclk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(imx585->xclk)) {
> > + dev_err(dev, "failed to get xclk\n");
> > + return PTR_ERR(imx585->xclk);
> > + }
> > +
> > + imx585->xclk_freq = clk_get_rate(imx585->xclk);
> > +
> > + for (i = 0; i < ARRAY_SIZE(imx585_inck_table); ++i) {
> > + if (imx585_inck_table[i].xclk_hz == imx585->xclk_freq) {
> > + imx585->inck_sel_val = imx585_inck_table[i].inck_sel;
> > + break;
> > + }
> > + }
> > +
> > + if (i == ARRAY_SIZE(imx585_inck_table)) {
> > + dev_err(dev, "unsupported XCLK rate %u Hz\n",
> > + imx585->xclk_freq);
> > + return -EINVAL;
> > + }
> > +
> > + dev_info(dev, "XCLK %u Hz → INCK_SEL 0x%02x\n",
> > + imx585->xclk_freq, imx585->inck_sel_val);
> > +
> > + ret = imx585_get_regulators(imx585);
> > + if (ret) {
> > + dev_err(dev, "failed to get regulators\n");
> > + return ret;
> > + }
> > +
> > + /* Request optional enable pin */
> > + imx585->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > + GPIOD_OUT_HIGH);
> > +
> > + /*
> > + * The sensor must be powered for imx585_check_module_exists()
> > + * to be able to read register
> > + */
> > + ret = imx585_power_on(dev);
> > + if (ret)
> > + return ret;
> > +
> > + ret = imx585_check_module_exists(imx585);
> > + if (ret)
> > + goto error_power_off;
> > +
> > + imx585->has_ircut = false;
> > + imx585->ircut_client = NULL;
> > +
> > + if (of_property_read_bool(dev->of_node, "ircut-mode")) {
> > + np = of_parse_phandle(dev->of_node, "ircut-controller", 0);
Those properties are not defined in the binding either.
> > + if (np) {
> > + imx585->ircut_client = of_find_i2c_device_by_node(np);
> > + of_node_put(np);
> > + ret = imx585_ircut_write(imx585, 0x01);
> > + if (!ret) {
> > + imx585->has_ircut = true;
> > + dev_info(dev, "IR-cut controller present at 0x%02x\n",
> > + imx585->ircut_client->addr);
> > + } else {
> > + dev_info(dev,
> > + "Can't communication with IR-cut control, dropping\n");
> > + }
> > + }
> > + } else {
> > + dev_info(dev, "No IR-cut controller\n");
> > + }
> > +
> > + /* Initialize default format */
> > + imx585_set_default_format(imx585);
> > +
> > + /* Enable runtime PM and turn off the device */
> > + pm_runtime_set_active(dev);
> > + pm_runtime_enable(dev);
> > + pm_runtime_idle(dev);
>
> It would be best to use PM runtime autosuspend. See the imx296 driver
> for an example.
>
> > +
> > + /* This needs the pm runtime to be registered. */
> > + ret = imx585_init_controls(imx585);
> > + if (ret)
> > + goto error_pm_runtime;
> > +
> > + /* Initialize subdev */
> > + imx585->sd.internal_ops = &imx585_internal_ops;
> > + imx585->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > + V4L2_SUBDEV_FL_HAS_EVENTS;
> > + imx585->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > + /* Initialize source pads */
> > + imx585->pad[IMAGE_PAD].flags = MEDIA_PAD_FL_SOURCE;
> > + imx585->pad[METADATA_PAD].flags = MEDIA_PAD_FL_SOURCE;
> > +
> > + ret = media_entity_pads_init(&imx585->sd.entity, NUM_PADS, imx585->pad);
> > + if (ret) {
> > + dev_err(dev, "failed to init entity pads: %d\n", ret);
> > + goto error_handler_free;
> > + }
> > +
> > + ret = v4l2_async_register_subdev_sensor(&imx585->sd);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to register sensor sub-device: %d\n", ret);
> > + goto error_media_entity;
> > + }
> > +
> > + return 0;
> > +
> > +error_media_entity:
> > + media_entity_cleanup(&imx585->sd.entity);
> > +
> > +error_handler_free:
> > + imx585_free_controls(imx585);
> > +
> > +error_pm_runtime:
> > + pm_runtime_disable(&client->dev);
> > + pm_runtime_set_suspended(&client->dev);
> > +
> > +error_power_off:
> > + imx585_power_off(&client->dev);
> > +
> > + return ret;
> > +}
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
2025-07-02 6:38 ` [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor Will Whang
2025-07-02 9:28 ` Laurent Pinchart
@ 2025-07-04 8:08 ` Krzysztof Kozlowski
[not found] ` <CAFoNnry_BDeH9ERiDsncdpaH-f_qKqXyyM3e=M=j5ogJidU68g@mail.gmail.com>
1 sibling, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-04 8:08 UTC (permalink / raw)
To: Will Whang, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On 02/07/2025 08:38, Will Whang wrote:
> +
> +description:
> + IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
> +
> +properties:
> + compatible:
> + const: sony,imx585
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: xclk
> +
> + clock-frequency:
> + enum: [ 74250000, 37125000, 72000000, 27000000, 24000000 ]
Drop the property.
> +
> + reg:
reg always follows compatible.
> + maxItems: 1
> + description: I2C Address for IMX585
Drop description, redundant.
> +
> + VANA-supply:
Lower case.
> + description: Analog power supply (3.3V)
> +
> + VDDL-supply:
> + description: Interface power supply (1.8V)
> +
> + VDIG-supply:
> + description: Digital power supply (1.1V)
> +
> + reset-gpios:
> + description: Sensor reset (XCLR) GPIO
> + maxItems: 1
> +
> + port:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + additionalProperties: false
> +
> + properties:
> + endpoint:
> + $ref: /schemas/media/video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + data-lanes:
> + anyOf:
> + - items:
> + - const: 1
> + - const: 2
> + - const: 3
> + - const: 4
> +
> + sync-mode:
Missing vendor prefix
> + description: |
> + Select the synchronisation mode of the sensor
> + 0 – internal sync, leader (default)
> + 1 – internal sync, follower
> + 2 – external sync
> + $ref: /schemas/types.yaml#/definitions/uint8
> + enum: [ 0, 1, 2 ]
This should be just string enum
Missing default
> +
> + link-frequencies:
> + description: Select the MIPI-CSI2 link speed in Mhz
> + items:
> + enum: [ 297000000, 360000000, 445500000, 594000000,
> + 720000000, 891000000, 1039500000 ]
> +
> + required:
> + - data-lanes
> + - link-frequencies
> +
> + required:
> + - endpoint
> +
> +required:
> + - compatible
> + - reg
And why here is different? BTW, here it is correct.
> + - clocks
> + - clock-frequency
Drop
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + imx585@1a {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "sony,imx585";
> + reg = <0x1a>;
> + clocks = <&imx585_clk>;
> + clock-frequency = <24000000>;
> +
> + VANA-supply = <&camera_vadd_3v3>;
> + VDDL-supply = <&camera_vdd1_1v8>;
> + VDIG-supply = <&camera_vdd2_1v1>;
> +
> + port {
> + imx585: endpoint {
> + remote-endpoint = <&cam>;
> + data-lanes = <1 2 3 4>;
> + link-frequencies = /bits/ 64 <720000000>;
> + };
> + };
> + };
> + };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index da34c7227..9cc404790 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23150,6 +23150,14 @@ T: git git://linuxtv.org/media.git
> F: Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
> F: drivers/media/i2c/imx415.c
>
> +SONY IMX585 SENSOR DRIVER
> +M: Will Whang <will@willwhang.com>
> +L: linux-media@vger.kernel.org
> +S: Maintained
> +T: git git://linuxtv.org/media.git
Drop, you do not have access there, AFAIK.
> +F: Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> +F: drivers/media/i2c/imx585.c
There is no such file.
> +
> SONY MEMORYSTICK SUBSYSTEM
> M: Maxim Levitsky <maximlevitsky@gmail.com>
> M: Alex Dubov <oakad@yahoo.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
[not found] ` <CAFoNnry_BDeH9ERiDsncdpaH-f_qKqXyyM3e=M=j5ogJidU68g@mail.gmail.com>
@ 2025-07-06 7:30 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-06 7:30 UTC (permalink / raw)
To: Will Whang
Cc: Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On 06/07/2025 08:52, Will Whang wrote:
>>> + link-frequencies:
>>> + description: Select the MIPI-CSI2 link speed in Mhz
>>> + items:
>>> + enum: [ 297000000, 360000000, 445500000, 594000000,
>>> + 720000000, 891000000, 1039500000 ]
>>> +
>>> + required:
>>> + - data-lanes
>>> + - link-frequencies
>>> +
>>> + required:
>>> + - endpoint
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>
>> And why here is different? BTW, here it is correct.
>>
> Sorry I'm not sure if I get your comment correctly but I think this updated
> version is what you mean?
I meant the order of properties listed here and in properties: section.
>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
[not found] ` <CAFoNnrxquDp_yx_HSOe00cVDMcw2G+rTZs8x8RgOD3RO=tq-XA@mail.gmail.com>
@ 2025-07-06 19:45 ` Laurent Pinchart
0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2025-07-06 19:45 UTC (permalink / raw)
To: Will Whang
Cc: Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi will,
On Sat, Jul 05, 2025 at 11:55:04PM -0700, Will Whang wrote:
> Hi Laurent,
>
> Thank you for the feedback, very much appreciated!
> Reply inline.
> (Resend this email for reply all)
Please reply in plain-text only, as HTML e-mails get filtered out by
kernel mailing lists.
> On Wed, Jul 2, 2025 at 2:37 AM Laurent Pinchart wrote:
> > On Wed, Jul 02, 2025 at 07:38:33AM +0100, Will Whang wrote:
> > > Document the devicetree binding for the Sony IMX585. The schema
> > > covers the CSI-2 data-lanes, the optional 'mono-mode' flag,
> > > and the internal-sync properties used by the driver.
> > >
> > > Signed-off-by: Will Whang <will@willwhang.com>
> > > ---
> > > .../bindings/media/i2c/sony,imx585.yaml | 120 ++++++++++++++++++
> > > MAINTAINERS | 8 ++
> > > 2 files changed, 128 insertions(+)
> > > create mode 100644
> > Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> > >
> > > diff --git
> > a/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> > b/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> > > new file mode 100644
> > > index 000000000..d050d1642
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> > > @@ -0,0 +1,120 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +# Copyright (C) 2024 Ideas on Board Oy
> >
> > Unless there's something I'm not aware of, I don't think Ideas on Board
> > wrote this. You can use your own copyright.
>
> Yeah sorry about this, I've updated this one.
>
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/i2c/sony,imx585.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Sony IMX585 Sensor
> > > +
> > > +maintainers:
> > > + - Will Whang <will@willwhang.com>
> > > +
> > > +description:
> > > + IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
> > > +
> >
> > You should add
> >
> > allOf:
> > - $ref: /schemas/media/video-interface-devices.yaml#
> >
> > here to support generic sensor properties. You will need to replace
> >
> > additionalProperties: false
> >
> > with
> >
> > unevaluatedProperties: false
> >
> > below.
>
> Updated.
You don't need to reply to individual review comments if you agree with
them and update the code. By only replying to the points that require
discussions, the conversation is easier to read.
> > > +properties:
> > > + compatible:
> > > + const: sony,imx585
> > > +
> > > + clocks:
> > > + maxItems: 1
> > > +
> > > + clock-names:
> > > + const: xclk
> >
> > When there's a single clock you can drop clock-names.
> >
> Updated.
>
> > > +
> > > + clock-frequency:
> > > + enum: [ 74250000, 37125000, 72000000, 27000000, 24000000 ]
> >
> > The clock-frequency property is frowned upon for sensors in DT. If the
> > aim is to set the frequency of the clock, it should be done through
> > assigned-clocks and assigned-clock-rates. If the aim is to convey the
> > clock frequency to the driver, it should be done by calling
> > clk_get_rate() in the driver.
>
> The aim is to set the frequency for the driver to handle different clock frequencies,
> currently the driver is using clk_get_rate() but because it only supports
> these frequencies I thought I need to list the valid options here.
There's no need to. DT writers are expected to know about the hardware,
you don't need to document here what frequencies the sensor supports. DT
properties are meant to convey information to drivers, and drivers don't
need the clock-frequency property as they can get the clock rate (set
from assigned-clock-rates) from the system.
> > > +
> > > + reg:
> > > + maxItems: 1
> > > + description: I2C Address for IMX585
> >
> > You can drop the description, it's always the same for I2C devices.
>
> Updated.
>
> > > +
> > > + VANA-supply:
> > > + description: Analog power supply (3.3V)
> > > +
> > > + VDDL-supply:
> > > + description: Interface power supply (1.8V)
> > > +
> > > + VDIG-supply:
> > > + description: Digital power supply (1.1V)
> > > +
> > > + reset-gpios:
> > > + description: Sensor reset (XCLR) GPIO
> > > + maxItems: 1
> > > +
> > > + port:
> > > + $ref: /schemas/graph.yaml#/$defs/port-base
> > > + additionalProperties: false
> > > +
> > > + properties:
> > > + endpoint:
> > > + $ref: /schemas/media/video-interfaces.yaml#
> > > + unevaluatedProperties: false
> > > +
> > > + properties:
> > > + data-lanes:
> > > + anyOf:
> > > + - items:
> > > + - const: 1
> > > + - const: 2
> > > + - const: 3
> > > + - const: 4
> >
> > Does that mean that the sensor supports data lane remapping ? I don't
> > see it implemented by the driver. If it's not supported by the hardware,
> > you should use
> >
> > properties:
> > data-lanes:
> > minItems: 1
> > items:
> > - const: 1
> > - const: 2
> > - const: 3
> > - const: 4
> >
> > To guarantee the order.
>
> The driver can only support either 2-lane or 4-lane mode. Updated.
>
> > > +
> > > + sync-mode:
> > > + description: |
> > > + Select the synchronisation mode of the sensor
> > > + 0 – internal sync, leader (default)
> > > + 1 – internal sync, follower
> > > + 2 – external sync
> > > + $ref: /schemas/types.yaml#/definitions/uint8
> > > + enum: [ 0, 1, 2 ]
> >
> > This seems to be a sensor-level property, not an endpoint property. As
> > it's not standard, it should also have a vendor prefix, i.e.
> > sony,sync-mode. I'm wondering, though, if we shouldn't try to
> > standardize it in video-interface-devices.yaml.
>
> Updated, along with the feedback from Krzysztof, I've make the following
> modifications:
> sony,sync-mode:
> description:
> Select the global synchronisation mode of the sensor.
> $ref: /schemas/types.yaml#/definitions/string
> enum:
> - internal-leader
> - internal-follower
> - external
> default: internal-leader
I'll comment on that in v2, when reading how the driver handles external
triggers and synchronization.
> > +
> > > + link-frequencies:
> > > + description: Select the MIPI-CSI2 link speed in Mhz
> >
> > You can drop the description, it's already described in
> > video-interfaces.yaml.
>
> Updated.
>
> > > + items:
> > > + enum: [ 297000000, 360000000, 445500000, 594000000,
> > > + 720000000, 891000000, 1039500000 ]
> >
> > Are those frequencies the only ones the hardware can support, or do they
> > come from the driver only supporting a fixed set of sensor PLL
> > configurations ? In the latter case I would drop the enumeration.
>
> Yes, these are the ones that it supports.
>
> > > +
> > > + required:
> > > + - data-lanes
> > > + - link-frequencies
> > > +
> > > + required:
> > > + - endpoint
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - clocks
> > > + - clock-frequency
> > > + - port
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + i2c {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + imx585@1a {
> > > + compatible = "sony,imx585";
> > > + reg = <0x1a>;
> > > + clocks = <&imx585_clk>;
> > > + clock-frequency = <24000000>;
> > > +
> > > + VANA-supply = <&camera_vadd_3v3>;
> > > + VDDL-supply = <&camera_vdd1_1v8>;
> > > + VDIG-supply = <&camera_vdd2_1v1>;
> > > +
> > > + port {
> > > + imx585: endpoint {
> > > + remote-endpoint = <&cam>;
> > > + data-lanes = <1 2 3 4>;
> > > + link-frequencies = /bits/ 64 <720000000>;
> > > + };
> > > + };
> > > + };
> > > + };
> > > +...
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index da34c7227..9cc404790 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -23150,6 +23150,14 @@ T: git git://linuxtv.org/media.git
> > > F: Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
> > > F: drivers/media/i2c/imx415.c
> > >
> > > +SONY IMX585 SENSOR DRIVER
> > > +M: Will Whang <will@willwhang.com>
> > > +L: linux-media@vger.kernel.org
> > > +S: Maintained
> > > +T: git git://linuxtv.org/media.git
> > > +F: Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> > > +F: drivers/media/i2c/imx585.c
> > > +
> > > SONY MEMORYSTICK SUBSYSTEM
> > > M: Maxim Levitsky <maximlevitsky@gmail.com>
> > > M: Alex Dubov <oakad@yahoo.com>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 3/4] media: i2c: imx585: Add Sony IMX585 image-sensor driver
[not found] ` <CAFoNnrx-YpQwY6_908x=8LK1uwWw0y5zKxsv+aTsW1fxX554vg@mail.gmail.com>
@ 2025-07-06 20:30 ` Laurent Pinchart
0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2025-07-06 20:30 UTC (permalink / raw)
To: Will Whang
Cc: Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:SONY IMX585 SENSOR DRIVER, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Will,
On Sun, Jul 06, 2025 at 12:09:26AM -0700, Will Whang wrote:
> Hi Laurent,
>
> Thanks for the feedback, I really appreciate it.
> Most if not all the feedback is straightforward to fix, but I do encounter
> a few problems when trying to incorporate all the feedback
> Reply inline.
> (resend this email for reply all)
> Thanks,
> WillWhang
>
> On Thu, Jul 3, 2025 at 10:51 AM Laurent Pinchart wrote:
> > Hi Will,
> >
> > Thank you for the patch.
> >
> > Here's a first review, focussing on API usage and coding style.
> >
> > On Wed, Jul 02, 2025 at 07:38:35AM +0100, Will Whang wrote:
> > > Implements support for:
> > > * 4-lane / 2-lane CSI-2
> > > * 12-bit linear, 12-bit HDR-GC and 16-bit Clear-HDR modes
> > > * Mono variant switch, HCG, custom HDR controls
> > > * Tested on Raspberry Pi 4/5 with 24 MHz XCLK.
> > >
> > > Signed-off-by: Will Whang <will@willwhang.com>
> > > ---
> > > drivers/media/i2c/Kconfig | 9 +
> > > drivers/media/i2c/Makefile | 1 +
> > > drivers/media/i2c/imx585.c | 2466 ++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 2476 insertions(+)
> > > create mode 100644 drivers/media/i2c/imx585.c
[snip]
> > > diff --git a/drivers/media/i2c/imx585.c b/drivers/media/i2c/imx585.c
> > > new file mode 100644
> > > index 000000000..2c4212290
> > > --- /dev/null
> > > +++ b/drivers/media/i2c/imx585.c
> > > @@ -0,0 +1,2466 @@
[snip]
> > > +struct imx585 {
> > > + struct v4l2_subdev sd;
> > > + struct media_pad pad[NUM_PADS];
> > > +
> > > + unsigned int fmt_code;
> >
> > Please use the v4l2_subdev active state API to replace this. You will
> > need to call v4l2_subdev_init_finalize(), and the V4L2 framework will
> > allocate a v4l2_subdev_state instance for the active state. Most subdev
> > operations won't have to test for V4L2_SUBDEV_FORMAT_ACTIVE or
> > V4L2_SUBDEV_FORMAT_TRY, and will simply store data in the state passed
> > to the function. See commit e8a5b1df000e ("media: i2c: imx219: Use
> > subdev active state") for an example.
>
> I've tried to update the code to use active state API looking at the
> examples but I keep getting kernel error on
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000008
> [ 3.830058] imx585_set_ctrl+0x58/0xc10 [imx585]
> [ 3.834689] try_or_set_cluster+0x19c/0x2e0 [videodev]
> [ 3.839859] set_ctrl+0xb4/0x100 [videodev]
> [ 3.844066] __v4l2_ctrl_modify_range+0x144/0x1e8 [videodev]
> [ 3.849756] imx585_set_framing_limits+0x180/0x298 [imx585]
> [ 3.855347] imx585_probe+0x980/0xb28 [imx585]
>
> When probing, the driver tries to update the pixel rate with
> __v4l2_ctrl_modify_range but s_ctrl is dereferencing a NULL pointer
> somewhere here in imx585_set_ctrl.
>
> state = v4l2_subdev_get_locked_active_state(&imx585->sd);
> fmt = v4l2_subdev_state_get_format(state, 0);
> get_mode_table(imx585, fmt->code, &mode_list, &num_modes);
> mode = v4l2_find_nearest_size(mode_list, num_modes, width, height,
> fmt->width, fmt->height);
You will need to move the imx585_set_framing_limits() call from
imx585_init_controls() to a later point at probe time, after
v4l2_subdev_init_finalize().
> Is it really required to use active state API? I feel like the modification
> I've done is just replacing one line of code with a few more just to get
> the same imx585->mode and imx585->fmt_code.
Yes, all new drivers should use it. It is required to support new
features, and will increasingly become a mandatory building block as we
move forward.
> > > +
> > > + struct clk *xclk;
> > > + u32 xclk_freq;
> > > +
> > > + /* chosen INCK_SEL register value */
> > > + u8 inck_sel_val;
> > > +
> > > + /* Link configurations */
> > > + unsigned int lane_count;
> > > + unsigned int link_freq_idx;
> > > +
> > > + struct gpio_desc *reset_gpio;
> > > + struct regulator_bulk_data supplies[imx585_NUM_SUPPLIES];
> > > +
> > > + struct v4l2_ctrl_handler ctrl_handler;
> > > +
> > > + /* V4L2 Controls */
> > > + struct v4l2_ctrl *pixel_rate;
> > > + struct v4l2_ctrl *link_freq;
> > > + struct v4l2_ctrl *exposure;
> > > + struct v4l2_ctrl *gain;
> > > + struct v4l2_ctrl *hcg_ctrl;
> > > + struct v4l2_ctrl *vflip;
> > > + struct v4l2_ctrl *hflip;
> > > + struct v4l2_ctrl *vblank;
> > > + struct v4l2_ctrl *hblank;
> > > + struct v4l2_ctrl *blacklevel;
> > > +
> > > + /* V4L2 HDR Controls */
> > > + struct v4l2_ctrl *hdr_mode;
> > > + struct v4l2_ctrl *datasel_th_ctrl;
> > > + struct v4l2_ctrl *datasel_bk_ctrl;
> > > + struct v4l2_ctrl *gdc_th_ctrl;
> > > + struct v4l2_ctrl *gdc_exp_ctrl_l;
> > > + struct v4l2_ctrl *gdc_exp_ctrl_h;
> > > + struct v4l2_ctrl *hdr_gain_ctrl;
> > > +
> > > + /* V4L2 IR Cut filter switch Controls */
> > > + bool has_ircut;
> > > + struct v4l2_ctrl *ircut_ctrl;
> > > + struct i2c_client *ircut_client;
> > > +
> > > + /* Current mode */
> > > + const struct imx585_mode *mode;
> >
> > This should be dropped too, the imx585_mode instance should be lookup up
> > based on the v4l2_subdev_state.
>
> > > +
> > > + /* HCG enabled flag*/
> > > + bool hcg;
> > > +
> > > + /* Mono mode */
> > > + bool mono;
> > > +
> > > + /* Clear HDR mode */
> > > + bool clear_HDR;
> >
> > All variables must be lower case.
> >
> Updated.
>
> > > +
> > > + /* Sync Mode*/
> > > + /* 0 = Internal Sync Leader Mode
> > > + * 1 = External Sync Leader Mode
> > > + * 2 = Follower Mode
> > > + * The datasheet wording is very confusing but basically:
> > > + * Leader Mode = Sensor using internal clock to drive the sensor
> > > + * But with external sync mode you can send a XVS input so the sensor
> > > + * will try to align with it.
> > > + * For Follower mode it is purely driven by external clock.
> > > + * In this case you need to drive both XVS and XHS.
> > > + */
> > > + u32 sync_mode;
> > > +
> > > + /* Tracking sensor VMAX/HMAX value */
> > > + u16 HMAX;
> > > + u32 VMAX;
> > > +
> > > + /*
> > > + * Mutex for serialized access:
> > > + * Protect sensor module set pad format and start/stop streaming safely.
> > > + */
> > > + struct mutex mutex;
> >
> > You will be able to drop this when switching to the v4l2_subdev active
> > state API.
> >
> > > +
> > > + /* Streaming on/off */
> > > + bool streaming;
> > > +
> > > + /* Rewrite common registers on stream on? */
> > > + bool common_regs_written;
> > > +};
[snip]
> > > +/* ------------------------------------------------------------------
> > > + * Optional IR-cut helper
> > > + * ------------------------------------------------------------------
> > > + */
> > > +
> > > +/* One-byte “command” sent to the IR-cut MCU at imx585->ircut_client
> > */
> >
> > Is that MCU integrated in the camera sensor, or in the camera module ?
> >
> In the camera module, a separate MCU is reading the instructions.
Then it should be handled by a separate driver, with a separate DT node.
[snip]
> > > +static int imx585_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> >
> > This should be dropped and replaced by the .init_state() operation.
> >
> Will do, is this related to the active state API?
It's needed for the active state API, but is recommended even without
it.
> > > +{
> > > + struct imx585 *imx585 = to_imx585(sd);
> > > + struct v4l2_mbus_framefmt *try_fmt_img =
> > > + v4l2_subdev_state_get_format(fh->state, IMAGE_PAD);
> > > + struct v4l2_mbus_framefmt *try_fmt_meta =
> > > + v4l2_subdev_state_get_format(fh->state, METADATA_PAD);
> > > + struct v4l2_rect *try_crop;
> > > +
> > > + mutex_lock(&imx585->mutex);
> > > +
> > > + /* Initialize try_fmt for the image pad */
> > > + try_fmt_img->width = supported_modes[0].width;
> > > + try_fmt_img->height = supported_modes[0].height;
> > > + if (imx585->mono)
> > > + try_fmt_img->code = imx585_get_format_code(imx585, MEDIA_BUS_FMT_Y12_1X12);
> > > + else
> > > + try_fmt_img->code = imx585_get_format_code(imx585, MEDIA_BUS_FMT_SRGGB12_1X12);
> > > +
> > > + try_fmt_img->field = V4L2_FIELD_NONE;
> > > +
> > > + /* Initialize try_fmt for the embedded metadata pad */
> > > + try_fmt_meta->width = IMX585_EMBEDDED_LINE_WIDTH;
> > > + try_fmt_meta->height = IMX585_NUM_EMBEDDED_LINES;
> > > + try_fmt_meta->code = MEDIA_BUS_FMT_SENSOR_DATA;
> > > + try_fmt_meta->field = V4L2_FIELD_NONE;
> > > +
> > > + /* Initialize try_crop */
> > > + try_crop = v4l2_subdev_state_get_crop(fh->state, IMAGE_PAD);
> > > + try_crop->left = IMX585_PIXEL_ARRAY_LEFT;
> > > + try_crop->top = IMX585_PIXEL_ARRAY_TOP;
> > > + try_crop->width = IMX585_PIXEL_ARRAY_WIDTH;
> > > + try_crop->height = IMX585_PIXEL_ARRAY_HEIGHT;
> > > +
> > > + mutex_unlock(&imx585->mutex);
> > > +
> > > + return 0;
> > > +}
[snip]
> > > +static void imx585_update_hmax(struct imx585 *imx585)
> > > +{
> > > + struct i2c_client *client = v4l2_get_subdevdata(&imx585->sd);
> >
> > Store the struct device pointer in struct imx585 and use it instead of
> > client->dev. This should drop most usages of i2c_client from the driver.
> >
> Updated.
>
> > > +
> > > + const u32 base_4lane = HMAX_table_4lane_4K[imx585->link_freq_idx];
> > > + const u32 lane_scale = (imx585->lane_count == 2) ? 2 : 1;
> > > + const u32 factor = base_4lane * lane_scale;
> > > + const u32 hdr_factor = (imx585->clear_HDR) ? 2 : 1;
> > > +
> > > + dev_info(&client->dev, "Upadte minimum HMAX\n");
> > > + dev_info(&client->dev, "\tbase_4lane: %d\n", base_4lane);
> > > + dev_info(&client->dev, "\tlane_scale: %d\n", lane_scale);
> > > + dev_info(&client->dev, "\tfactor: %d\n", factor);
> > > + dev_info(&client->dev, "\thdr_factor: %d\n", hdr_factor);
> >
> > This makes the driver way too chatty. The messages should be demoted to
> > dev_dbg(), or dropped.
>
> Will do, but is there a recommended way to have a debug flag that will work
> without recompiling the linux kernel with dynamic debugging support?
> Raspberry Pi Kernel currently does not have it enabled by default so I want
> to have a way to support enabling full logging via either dev_dbg or module
> parameters.
I'm afraid you need to either enable dynamic debugging, or #define DEBUG
at the beginning of the file to get the dev_dbg() messages. Dynamic
debug is the recommended option.
> > > +
> > > + for (unsigned int i = 0; i < ARRAY_SIZE(supported_modes); ++i) {
> > > + u32 h = factor / supported_modes[i].hmax_div;
> > > + u64 v = IMX585_VMAX_DEFAULT * hdr_factor;
> > > +
> > > + supported_modes[i].min_HMAX = h;
> > > + supported_modes[i].default_HMAX = h;
> > > + supported_modes[i].min_VMAX = v;
> > > + supported_modes[i].default_VMAX = v;
> > > + dev_info(&client->dev, "\tvmax: %lld x hmax: %d\n", v, h);
> > > + }
> > > +}
[snip]
> > > +static const struct v4l2_subdev_video_ops imx585_video_ops = {
> > > + .s_stream = imx585_set_stream,
> >
> > Please implement the .enable_streams() and .disable_streams() pad
> > ops, and use v4l2_subdev_s_stream_helper here.
>
> Is this related to active API?
It's a preparation for future changes. The .s_stream() operation doesn't
properly support multiple streams, which will be required for embedded
data support. New drivers should use .enable_streams() and
.disable_streams(), and those operations indeed assume usage of the
active state API.
> > > +};
[snip]
> > > +static int imx585_check_hwcfg(struct device *dev, struct imx585 *imx585)
> > > +{
> > > + struct fwnode_handle *endpoint;
> >
> > struct fwnode_handle *endpoint __free(fwnode_handle_put) = NULL;
> >
> > and rop the fwnode_handle_put() call below. That will simplify error
> > handling.
>
> For some reason when I tried this, gcc showed an error for me.
Sorry, it should have been __free(fwnode_handle).
> > > + struct v4l2_fwnode_endpoint ep_cfg = {
> > > + .bus_type = V4L2_MBUS_CSI2_DPHY
> > > + };
> > > + int ret = -EINVAL;
> > > + int i;
> > > +
> > > + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
> > > + if (!endpoint) {
> > > + dev_err(dev, "endpoint node not found\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep_cfg)) {
> > > + dev_err(dev, "could not parse endpoint\n");
> > > + goto error_out;
> > > + }
> > > +
> > > + /* Check the number of MIPI CSI2 data lanes */
> > > + if (ep_cfg.bus.mipi_csi2.num_data_lanes != 2 && ep_cfg.bus.mipi_csi2.num_data_lanes != 4) {
> > > + dev_err(dev, "only 2 or 4 data lanes are currently supported\n");
> > > + goto error_out;
> > > + }
> > > + imx585->lane_count = ep_cfg.bus.mipi_csi2.num_data_lanes;
> > > + dev_info(dev, "Data lanes: %d\n", imx585->lane_count);
> > > +
> > > + /* Check the link frequency set in device tree */
> > > + if (!ep_cfg.nr_of_link_frequencies) {
> > > + dev_err(dev, "link-frequency property not found in DT\n");
> > > + goto error_out;
> > > + }
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(link_freqs); i++) {
> > > + if (link_freqs[i] == ep_cfg.link_frequencies[0]) {
> > > + imx585->link_freq_idx = i;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + if (i == ARRAY_SIZE(link_freqs)) {
> > > + dev_err(dev, "Link frequency not supported: %lld\n",
> > > + ep_cfg.link_frequencies[0]);
> > > + ret = -EINVAL;
> > > + goto error_out;
> > > + }
> > > +
> > > + dev_info(dev, "Link Speed: %lld Mhz\n", ep_cfg.link_frequencies[0]);
> > > +
> > > + ret = 0;
> > > +
> > > +error_out:
> > > + v4l2_fwnode_endpoint_free(&ep_cfg);
> > > + fwnode_handle_put(endpoint);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int imx585_probe(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct device_node *np;
> > > + struct imx585 *imx585;
> > > + const struct of_device_id *match;
> > > + int ret, i;
> > > + u32 sync_mode;
> > > +
> > > + imx585 = devm_kzalloc(&client->dev, sizeof(*imx585), GFP_KERNEL);
> > > + if (!imx585)
> > > + return -ENOMEM;
> > > +
> > > + v4l2_i2c_subdev_init(&imx585->sd, client, &imx585_subdev_ops);
> > > +
> > > + match = of_match_device(imx585_dt_ids, dev);
> > > + if (!match)
> > > + return -ENODEV;
> >
> > This doesn't seem needed.
>
> Updated.
>
> > > +
> > > + dev_info(dev, "Reading dtoverlay config:\n");
> > > + imx585->mono = of_property_read_bool(dev->of_node, "mono-mode");
> >
> > Mono sensors should be detected based on the compatible string, not with
> > a separate property.
>
> Updated.
> But will it cause issues for libcamera such that the ipa doesn't know which
> tuning file to use?
Userspace should be able to tell the models apart based on the reported
media bus code (MEDIA_BUS_FMT_Y12_1X12 vs. MEDIA_BUS_FMT_SGBRG12_1X12).
> > > + if (imx585->mono)
> > > + dev_info(dev, "Mono Mode Selected, make sure you have the correct sensor variant\n");
> > > +
> > > + imx585->clear_HDR = of_property_read_bool(dev->of_node,
> > "clearHDR-mode");
> >
> > This doesn't seem to belong to DT, as it's a runtime option.
>
> Updated, I removed the clearHDR-mode option in DT, was originally for setup
> the sensor with clearHDR mode directly.
>
> > > + dev_info(dev, "ClearHDR: %d\n", imx585->clear_HDR);
> > > +
> > > + imx585->sync_mode = 0;
> > > + ret = of_property_read_u32(dev->of_node, "sync-mode", &sync_mode);
> > > + if (!ret) {
> > > + if (sync_mode > 2) {
> > > + dev_warn(dev, "sync-mode out of range, using 0\n");
> > > + sync_mode = 0;
> > > + }
> > > + imx585->sync_mode = sync_mode;
> > > + } else if (ret != -EINVAL) { /* property present but bad */
> > > + dev_err(dev, "sync-mode malformed (%pe)\n", ERR_PTR(ret));
> > > + return ret;
> > > + }
> > > + dev_info(dev, "Sync Mode: %s\n", sync_mode_menu[imx585->sync_mode]);
> > > +
> > > + /* Check the hardware configuration in device tree */
> > > + if (imx585_check_hwcfg(dev, imx585))
> > > + return -EINVAL;
> > > +
> > > + /* Get system clock (xclk) */
> > > + imx585->xclk = devm_clk_get(dev, NULL);
> > > + if (IS_ERR(imx585->xclk)) {
> > > + dev_err(dev, "failed to get xclk\n");
> > > + return PTR_ERR(imx585->xclk);
> > > + }
> > > +
> > > + imx585->xclk_freq = clk_get_rate(imx585->xclk);
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(imx585_inck_table); ++i) {
> > > + if (imx585_inck_table[i].xclk_hz == imx585->xclk_freq) {
> > > + imx585->inck_sel_val = imx585_inck_table[i].inck_sel;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + if (i == ARRAY_SIZE(imx585_inck_table)) {
> > > + dev_err(dev, "unsupported XCLK rate %u Hz\n",
> > > + imx585->xclk_freq);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + dev_info(dev, "XCLK %u Hz → INCK_SEL 0x%02x\n",
> > > + imx585->xclk_freq, imx585->inck_sel_val);
> > > +
> > > + ret = imx585_get_regulators(imx585);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get regulators\n");
> > > + return ret;
> > > + }
> > > +
> > > + /* Request optional enable pin */
> > > + imx585->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > > + GPIOD_OUT_HIGH);
> > > +
> > > + /*
> > > + * The sensor must be powered for imx585_check_module_exists()
> > > + * to be able to read register
> > > + */
> > > + ret = imx585_power_on(dev);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = imx585_check_module_exists(imx585);
> > > + if (ret)
> > > + goto error_power_off;
> > > +
> > > + imx585->has_ircut = false;
> > > + imx585->ircut_client = NULL;
> > > +
> > > + if (of_property_read_bool(dev->of_node, "ircut-mode")) {
> > > + np = of_parse_phandle(dev->of_node, "ircut-controller", 0);
> > > + if (np) {
> > > + imx585->ircut_client = of_find_i2c_device_by_node(np);
> > > + of_node_put(np);
> > > + ret = imx585_ircut_write(imx585, 0x01);
> > > + if (!ret) {
> > > + imx585->has_ircut = true;
> > > + dev_info(dev, "IR-cut controller present at 0x%02x\n",
> > > + imx585->ircut_client->addr);
> > > + } else {
> > > + dev_info(dev,
> > > + "Can't communication with IR-cut control, dropping\n");
> > > + }
> > > + }
> > > + } else {
> > > + dev_info(dev, "No IR-cut controller\n");
> > > + }
> > > +
> > > + /* Initialize default format */
> > > + imx585_set_default_format(imx585);
> > > +
> > > + /* Enable runtime PM and turn off the device */
> > > + pm_runtime_set_active(dev);
> > > + pm_runtime_enable(dev);
> > > + pm_runtime_idle(dev);
> >
> > It would be best to use PM runtime autosuspend. See the imx296 driver
> > for an example.
>
> Will update for V2.
>
> > > +
> > > + /* This needs the pm runtime to be registered. */
> > > + ret = imx585_init_controls(imx585);
> > > + if (ret)
> > > + goto error_pm_runtime;
> > > +
> > > + /* Initialize subdev */
> > > + imx585->sd.internal_ops = &imx585_internal_ops;
> > > + imx585->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > > + V4L2_SUBDEV_FL_HAS_EVENTS;
> > > + imx585->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > +
> > > + /* Initialize source pads */
> > > + imx585->pad[IMAGE_PAD].flags = MEDIA_PAD_FL_SOURCE;
> > > + imx585->pad[METADATA_PAD].flags = MEDIA_PAD_FL_SOURCE;
> > > +
> > > + ret = media_entity_pads_init(&imx585->sd.entity, NUM_PADS, imx585->pad);
> > > + if (ret) {
> > > + dev_err(dev, "failed to init entity pads: %d\n", ret);
> > > + goto error_handler_free;
> > > + }
> > > +
> > > + ret = v4l2_async_register_subdev_sensor(&imx585->sd);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to register sensor sub-device: %d\n", ret);
> > > + goto error_media_entity;
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +error_media_entity:
> > > + media_entity_cleanup(&imx585->sd.entity);
> > > +
> > > +error_handler_free:
> > > + imx585_free_controls(imx585);
> > > +
> > > +error_pm_runtime:
> > > + pm_runtime_disable(&client->dev);
> > > + pm_runtime_set_suspended(&client->dev);
> > > +
> > > +error_power_off:
> > > + imx585_power_off(&client->dev);
> > > +
> > > + return ret;
> > > +}
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-07-06 20:30 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-02 6:38 [PATCH v1 0/4] media: Add Sony IMX585 image sensor support Will Whang
2025-07-02 6:38 ` [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor Will Whang
2025-07-02 9:28 ` Laurent Pinchart
[not found] ` <CAFoNnrxquDp_yx_HSOe00cVDMcw2G+rTZs8x8RgOD3RO=tq-XA@mail.gmail.com>
2025-07-06 19:45 ` Laurent Pinchart
2025-07-04 8:08 ` Krzysztof Kozlowski
[not found] ` <CAFoNnry_BDeH9ERiDsncdpaH-f_qKqXyyM3e=M=j5ogJidU68g@mail.gmail.com>
2025-07-06 7:30 ` Krzysztof Kozlowski
2025-07-02 6:38 ` [PATCH v1 2/4] media: uapi: Add custom IMX585 control IDs Will Whang
2025-07-02 6:38 ` [PATCH v1 3/4] media: i2c: imx585: Add Sony IMX585 image-sensor driver Will Whang
2025-07-03 17:51 ` Laurent Pinchart
2025-07-03 17:54 ` Laurent Pinchart
[not found] ` <CAFoNnrx-YpQwY6_908x=8LK1uwWw0y5zKxsv+aTsW1fxX554vg@mail.gmail.com>
2025-07-06 20:30 ` Laurent Pinchart
2025-07-02 6:38 ` [PATCH v1 4/4] media: docs: Add userspace-API guide for the IMX585 driver Will Whang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).