* [PATCH v2 01/20] media: dt-bindings: Convert imx290.txt to YAML
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 15:05 ` Krzysztof Kozlowski
2022-10-17 13:57 ` Dave Stevenson
2022-10-16 6:15 ` [PATCH v2 02/20] media: i2c: imx290: Use device lock for the control handler Laurent Pinchart
` (18 subsequent siblings)
19 siblings, 2 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson, Rob Herring, Krzysztof Kozlowski, devicetree
Convert the Sony IMX290 DT binding from text to YAML. Add Manivannan as
a maintainer given that he is listed in MAINTAINERS for the file, as
volunteering myself.
The name of the input clock, "xclk", is wrong as the hardware manual
names it INCK. As the device has a single clock, the name could be
omitted, but that would require a corresponding change to the driver and
is thus a candidate for further patches.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../devicetree/bindings/media/i2c/imx290.txt | 57 --------
.../bindings/media/i2c/sony,imx290.yaml | 129 ++++++++++++++++++
MAINTAINERS | 2 +-
3 files changed, 130 insertions(+), 58 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/media/i2c/imx290.txt
create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/imx290.txt b/Documentation/devicetree/bindings/media/i2c/imx290.txt
deleted file mode 100644
index a3cc21410f7c..000000000000
--- a/Documentation/devicetree/bindings/media/i2c/imx290.txt
+++ /dev/null
@@ -1,57 +0,0 @@
-* Sony IMX290 1/2.8-Inch CMOS Image Sensor
-
-The Sony IMX290 is a 1/2.8-Inch CMOS Solid-state image sensor with
-Square Pixel for Color Cameras. It is programmable through I2C and 4-wire
-interfaces. The sensor output is available via CMOS logic parallel SDR output,
-Low voltage LVDS DDR output and CSI-2 serial data output. The CSI-2 bus is the
-default. No bindings have been defined for the other busses.
-
-Required Properties:
-- compatible: Should be "sony,imx290"
-- reg: I2C bus address of the device
-- clocks: Reference to the xclk clock.
-- clock-names: Should be "xclk".
-- clock-frequency: Frequency of the xclk clock in Hz.
-- vdddo-supply: Sensor digital IO regulator.
-- vdda-supply: Sensor analog regulator.
-- vddd-supply: Sensor digital core regulator.
-
-Optional Properties:
-- reset-gpios: Sensor reset GPIO
-
-The imx290 device node should contain one 'port' child node with
-an 'endpoint' subnode. For further reading on port node refer to
-Documentation/devicetree/bindings/media/video-interfaces.txt.
-
-Required Properties on endpoint:
-- data-lanes: check ../video-interfaces.txt
-- link-frequencies: check ../video-interfaces.txt
-- remote-endpoint: check ../video-interfaces.txt
-
-Example:
- &i2c1 {
- ...
- imx290: camera-sensor@1a {
- compatible = "sony,imx290";
- reg = <0x1a>;
-
- reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
- pinctrl-names = "default";
- pinctrl-0 = <&camera_rear_default>;
-
- clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
- clock-names = "xclk";
- clock-frequency = <37125000>;
-
- vdddo-supply = <&camera_vdddo_1v8>;
- vdda-supply = <&camera_vdda_2v8>;
- vddd-supply = <&camera_vddd_1v5>;
-
- port {
- imx290_ep: endpoint {
- data-lanes = <1 2 3 4>;
- link-frequencies = /bits/ 64 <445500000>;
- remote-endpoint = <&csiphy0_ep>;
- };
- };
- };
diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
new file mode 100644
index 000000000000..21377daae026
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
@@ -0,0 +1,129 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/sony,imx290.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony IMX290 1/2.8-Inch CMOS Image Sensor
+
+maintainers:
+ - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
+ - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+
+description: |-
+ The Sony IMX290 is a 1/2.8-Inch CMOS Solid-state image sensor with Square
+ Pixel for Color Cameras. It is programmable through I2C and 4-wire
+ interfaces. The sensor output is available via CMOS logic parallel SDR
+ output, Low voltage LVDS DDR output and CSI-2 serial data output. The CSI-2
+ bus is the default. No bindings have been defined for the other busses.
+
+properties:
+ compatible:
+ enum:
+ - sony,imx290
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ description: Input clock (37.125 MHz or 74.25 MHz)
+ items:
+ - const: xclk
+
+ clock-frequency:
+ description: Frequency of the xclk clock in Hz
+
+ vdda-supply:
+ description: Analog power supply (2.9V)
+
+ vddd-supply:
+ description: Digital core power supply (1.2V)
+
+ vdddo-supply:
+ description: Digital I/O power supply (1.8V)
+
+ reset-gpios:
+ description: Sensor reset (XCLR) GPIO
+ maxItems: 1
+
+ port:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ description: |
+ Video output port
+
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ data-lanes:
+ anyOf:
+ - items:
+ - const: 1
+ - const: 2
+ - items:
+ - const: 1
+ - const: 2
+ - const: 3
+ - const: 4
+
+ link-frequencies: true
+
+ required:
+ - data-lanes
+ - link-frequencies
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - clock-frequency
+ - vdda-supply
+ - vddd-supply
+ - vdddo-supply
+ - port
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ imx290: camera-sensor@1a {
+ compatible = "sony,imx290";
+ reg = <0x1a>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&camera_rear_default>;
+
+ clocks = <&gcc 90>;
+ clock-names = "xclk";
+ clock-frequency = <37125000>;
+
+ vdddo-supply = <&camera_vdddo_1v8>;
+ vdda-supply = <&camera_vdda_2v8>;
+ vddd-supply = <&camera_vddd_1v5>;
+
+ reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
+
+ port {
+ imx290_ep: endpoint {
+ data-lanes = <1 2 3 4>;
+ link-frequencies = /bits/ 64 <445500000>;
+ remote-endpoint = <&csiphy0_ep>;
+ };
+ };
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 72b9654f764c..c431fd20e89b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18982,7 +18982,7 @@ M: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
L: linux-media@vger.kernel.org
S: Maintained
T: git git://linuxtv.org/media_tree.git
-F: Documentation/devicetree/bindings/media/i2c/imx290.txt
+F: Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
F: drivers/media/i2c/imx290.c
SONY IMX319 SENSOR DRIVER
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 01/20] media: dt-bindings: Convert imx290.txt to YAML
2022-10-16 6:15 ` [PATCH v2 01/20] media: dt-bindings: Convert imx290.txt to YAML Laurent Pinchart
@ 2022-10-16 15:05 ` Krzysztof Kozlowski
2022-10-17 13:57 ` Dave Stevenson
1 sibling, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-16 15:05 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson, Rob Herring, Krzysztof Kozlowski, devicetree
On 16/10/2022 02:15, Laurent Pinchart wrote:
> Convert the Sony IMX290 DT binding from text to YAML. Add Manivannan as
> a maintainer given that he is listed in MAINTAINERS for the file, as
> volunteering myself.
>
> The name of the input clock, "xclk", is wrong as the hardware manual
> names it INCK. As the device has a single clock, the name could be
> omitted, but that would require a corresponding change to the driver and
> is thus a candidate for further patches.
>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 01/20] media: dt-bindings: Convert imx290.txt to YAML
2022-10-16 6:15 ` [PATCH v2 01/20] media: dt-bindings: Convert imx290.txt to YAML Laurent Pinchart
2022-10-16 15:05 ` Krzysztof Kozlowski
@ 2022-10-17 13:57 ` Dave Stevenson
2022-10-25 14:12 ` Sakari Ailus
1 sibling, 1 reply; 29+ messages in thread
From: Dave Stevenson @ 2022-10-17 13:57 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Rob Herring, Krzysztof Kozlowski, devicetree
Hi Laurent
On Sun, 16 Oct 2022 at 07:15, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Convert the Sony IMX290 DT binding from text to YAML. Add Manivannan as
> a maintainer given that he is listed in MAINTAINERS for the file, as
> volunteering myself.
>
> The name of the input clock, "xclk", is wrong as the hardware manual
> names it INCK. As the device has a single clock, the name could be
> omitted, but that would require a corresponding change to the driver and
> is thus a candidate for further patches.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> .../devicetree/bindings/media/i2c/imx290.txt | 57 --------
> .../bindings/media/i2c/sony,imx290.yaml | 129 ++++++++++++++++++
> MAINTAINERS | 2 +-
> 3 files changed, 130 insertions(+), 58 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/media/i2c/imx290.txt
> create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/imx290.txt b/Documentation/devicetree/bindings/media/i2c/imx290.txt
> deleted file mode 100644
> index a3cc21410f7c..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/imx290.txt
> +++ /dev/null
> @@ -1,57 +0,0 @@
> -* Sony IMX290 1/2.8-Inch CMOS Image Sensor
> -
> -The Sony IMX290 is a 1/2.8-Inch CMOS Solid-state image sensor with
> -Square Pixel for Color Cameras. It is programmable through I2C and 4-wire
> -interfaces. The sensor output is available via CMOS logic parallel SDR output,
> -Low voltage LVDS DDR output and CSI-2 serial data output. The CSI-2 bus is the
> -default. No bindings have been defined for the other busses.
> -
> -Required Properties:
> -- compatible: Should be "sony,imx290"
> -- reg: I2C bus address of the device
> -- clocks: Reference to the xclk clock.
> -- clock-names: Should be "xclk".
> -- clock-frequency: Frequency of the xclk clock in Hz.
> -- vdddo-supply: Sensor digital IO regulator.
> -- vdda-supply: Sensor analog regulator.
> -- vddd-supply: Sensor digital core regulator.
> -
> -Optional Properties:
> -- reset-gpios: Sensor reset GPIO
> -
> -The imx290 device node should contain one 'port' child node with
> -an 'endpoint' subnode. For further reading on port node refer to
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Required Properties on endpoint:
> -- data-lanes: check ../video-interfaces.txt
> -- link-frequencies: check ../video-interfaces.txt
> -- remote-endpoint: check ../video-interfaces.txt
> -
> -Example:
> - &i2c1 {
> - ...
> - imx290: camera-sensor@1a {
> - compatible = "sony,imx290";
> - reg = <0x1a>;
> -
> - reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
> - pinctrl-names = "default";
> - pinctrl-0 = <&camera_rear_default>;
> -
> - clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
> - clock-names = "xclk";
> - clock-frequency = <37125000>;
> -
> - vdddo-supply = <&camera_vdddo_1v8>;
> - vdda-supply = <&camera_vdda_2v8>;
> - vddd-supply = <&camera_vddd_1v5>;
> -
> - port {
> - imx290_ep: endpoint {
> - data-lanes = <1 2 3 4>;
> - link-frequencies = /bits/ 64 <445500000>;
> - remote-endpoint = <&csiphy0_ep>;
> - };
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
> new file mode 100644
> index 000000000000..21377daae026
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/sony,imx290.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony IMX290 1/2.8-Inch CMOS Image Sensor
> +
> +maintainers:
> + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> + - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> +description: |-
> + The Sony IMX290 is a 1/2.8-Inch CMOS Solid-state image sensor with Square
> + Pixel for Color Cameras. It is programmable through I2C and 4-wire
> + interfaces. The sensor output is available via CMOS logic parallel SDR
> + output, Low voltage LVDS DDR output and CSI-2 serial data output. The CSI-2
> + bus is the default. No bindings have been defined for the other busses.
> +
> +properties:
> + compatible:
> + enum:
> + - sony,imx290
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + description: Input clock (37.125 MHz or 74.25 MHz)
> + items:
> + - const: xclk
> +
> + clock-frequency:
> + description: Frequency of the xclk clock in Hz
> +
> + vdda-supply:
> + description: Analog power supply (2.9V)
> +
> + vddd-supply:
> + description: Digital core power supply (1.2V)
> +
> + vdddo-supply:
> + description: Digital I/O power supply (1.8V)
> +
> + reset-gpios:
> + description: Sensor reset (XCLR) GPIO
> + maxItems: 1
> +
> + port:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + description: |
> + Video output port
> +
> + properties:
> + endpoint:
> + $ref: /schemas/media/video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + data-lanes:
> + anyOf:
> + - items:
> + - const: 1
> + - const: 2
> + - items:
> + - const: 1
> + - const: 2
> + - const: 3
> + - const: 4
> +
> + link-frequencies: true
> +
> + required:
> + - data-lanes
> + - link-frequencies
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - clock-frequency
> + - vdda-supply
> + - vddd-supply
> + - vdddo-supply
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + imx290: camera-sensor@1a {
> + compatible = "sony,imx290";
> + reg = <0x1a>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&camera_rear_default>;
> +
> + clocks = <&gcc 90>;
> + clock-names = "xclk";
> + clock-frequency = <37125000>;
> +
> + vdddo-supply = <&camera_vdddo_1v8>;
> + vdda-supply = <&camera_vdda_2v8>;
> + vddd-supply = <&camera_vddd_1v5>;
> +
> + reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
> +
> + port {
> + imx290_ep: endpoint {
> + data-lanes = <1 2 3 4>;
> + link-frequencies = /bits/ 64 <445500000>;
Minor nit that this won't work with the current Linux driver due to a
driver restriction implementing the recommended settings from Sony.
OV8865 has the same restrictions and notes it in the binding[1]. I
don't know if this is the preferred approach or not.
Dave
[1] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/media/i2c/ov8856.yaml#L78
> + remote-endpoint = <&csiphy0_ep>;
> + };
> + };
> + };
> + };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 72b9654f764c..c431fd20e89b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18982,7 +18982,7 @@ M: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> L: linux-media@vger.kernel.org
> S: Maintained
> T: git git://linuxtv.org/media_tree.git
> -F: Documentation/devicetree/bindings/media/i2c/imx290.txt
> +F: Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
> F: drivers/media/i2c/imx290.c
>
> SONY IMX319 SENSOR DRIVER
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v2 01/20] media: dt-bindings: Convert imx290.txt to YAML
2022-10-17 13:57 ` Dave Stevenson
@ 2022-10-25 14:12 ` Sakari Ailus
0 siblings, 0 replies; 29+ messages in thread
From: Sakari Ailus @ 2022-10-25 14:12 UTC (permalink / raw)
To: Dave Stevenson
Cc: Laurent Pinchart, linux-media, Manivannan Sadhasivam,
Alexander Stein, Rob Herring, Krzysztof Kozlowski, devicetree
Hi Dave,
On Mon, Oct 17, 2022 at 02:57:58PM +0100, Dave Stevenson wrote:
> > + link-frequencies = /bits/ 64 <445500000>;
>
> Minor nit that this won't work with the current Linux driver due to a
> driver restriction implementing the recommended settings from Sony.
> OV8865 has the same restrictions and notes it in the binding[1]. I
> don't know if this is the preferred approach or not.
The DT describes hardware and is not bound by (current) driver limitations.
So I'd say that if the hardware supports multiple frequencies, the
supported ones need to be listed here.
I'd also like to merge the series, it's been out of tree for long already.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 02/20] media: i2c: imx290: Use device lock for the control handler
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 01/20] media: dt-bindings: Convert imx290.txt to YAML Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 03/20] media: i2c: imx290: Print error code when I2C transfer fails Laurent Pinchart
` (17 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
The link frequency and pixel rate controls are set without holding the
control handler lock, resulting in kernel warnings. As the value of
those controls depend on the format, the simplest fix is to use the
device lock for the control handler.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/media/i2c/imx290.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 99f2a50d39a4..d97a5fb1d501 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -1043,6 +1043,7 @@ static int imx290_probe(struct i2c_client *client)
imx290_entity_init_cfg(&imx290->sd, NULL);
v4l2_ctrl_handler_init(&imx290->ctrls, 4);
+ imx290->ctrls.lock = &imx290->lock;
v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
V4L2_CID_GAIN, 0, 72, 1, 0);
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 03/20] media: i2c: imx290: Print error code when I2C transfer fails
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 01/20] media: dt-bindings: Convert imx290.txt to YAML Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 02/20] media: i2c: imx290: Use device lock for the control handler Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 04/20] media: i2c: imx290: Replace macro with explicit ARRAY_SIZE() Laurent Pinchart
` (16 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Knowing why I2C transfers fail is useful for debugging. Extend the error
message to print the error code.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/media/i2c/imx290.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index d97a5fb1d501..64bd43813dbf 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -370,7 +370,8 @@ static inline int __always_unused imx290_read_reg(struct imx290 *imx290, u16 add
ret = regmap_read(imx290->regmap, addr, ®val);
if (ret) {
- dev_err(imx290->dev, "I2C read failed for addr: %x\n", addr);
+ dev_err(imx290->dev, "Failed to read register 0x%04x: %d\n",
+ addr, ret);
return ret;
}
@@ -385,7 +386,8 @@ static int imx290_write_reg(struct imx290 *imx290, u16 addr, u8 value)
ret = regmap_write(imx290->regmap, addr, value);
if (ret) {
- dev_err(imx290->dev, "I2C write failed for addr: %x\n", addr);
+ dev_err(imx290->dev, "Failed to write register 0x%04x: %d\n",
+ addr, ret);
return ret;
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 04/20] media: i2c: imx290: Replace macro with explicit ARRAY_SIZE()
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (2 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 03/20] media: i2c: imx290: Print error code when I2C transfer fails Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 05/20] media: i2c: imx290: Drop imx290_write_buffered_reg() Laurent Pinchart
` (15 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Use ARRAY_SIZE(imx290->supplies) for code that needs the size of the
array, instead of relying on the IMX290_NUM_SUPPLIES. The result is less
error-prone as it ties the size to the array.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/media/i2c/imx290.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 64bd43813dbf..0f32f391b2e7 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -790,10 +790,10 @@ static int imx290_get_regulators(struct device *dev, struct imx290 *imx290)
{
unsigned int i;
- for (i = 0; i < IMX290_NUM_SUPPLIES; i++)
+ for (i = 0; i < ARRAY_SIZE(imx290->supplies); i++)
imx290->supplies[i].supply = imx290_supply_name[i];
- return devm_regulator_bulk_get(dev, IMX290_NUM_SUPPLIES,
+ return devm_regulator_bulk_get(dev, ARRAY_SIZE(imx290->supplies),
imx290->supplies);
}
@@ -852,7 +852,8 @@ static int imx290_power_on(struct device *dev)
return ret;
}
- ret = regulator_bulk_enable(IMX290_NUM_SUPPLIES, imx290->supplies);
+ ret = regulator_bulk_enable(ARRAY_SIZE(imx290->supplies),
+ imx290->supplies);
if (ret) {
dev_err(dev, "Failed to enable regulators\n");
clk_disable_unprepare(imx290->xclk);
@@ -876,7 +877,7 @@ static int imx290_power_off(struct device *dev)
clk_disable_unprepare(imx290->xclk);
gpiod_set_value_cansleep(imx290->rst_gpio, 1);
- regulator_bulk_disable(IMX290_NUM_SUPPLIES, imx290->supplies);
+ regulator_bulk_disable(ARRAY_SIZE(imx290->supplies), imx290->supplies);
return 0;
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 05/20] media: i2c: imx290: Drop imx290_write_buffered_reg()
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (3 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 04/20] media: i2c: imx290: Replace macro with explicit ARRAY_SIZE() Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 06/20] media: i2c: imx290: Drop regmap cache Laurent Pinchart
` (14 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
The imx290_write_buffered_reg() function wraps a register write with
register hold, to enable changing multiple registers synchronously. It
is used for the gain only, which is an 8-bit register, defeating its
purpose.
The feature is useful, but should be implemented differently. Drop the
function for now, to prepare for a rework of register access.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/media/i2c/imx290.c | 32 +-------------------------------
1 file changed, 1 insertion(+), 31 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 0f32f391b2e7..5646f1704a1e 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -413,41 +413,11 @@ static int imx290_set_register_array(struct imx290 *imx290,
return 0;
}
-static int imx290_write_buffered_reg(struct imx290 *imx290, u16 address_low,
- u8 nr_regs, u32 value)
-{
- unsigned int i;
- int ret;
-
- ret = imx290_write_reg(imx290, IMX290_REGHOLD, 0x01);
- if (ret) {
- dev_err(imx290->dev, "Error setting hold register\n");
- return ret;
- }
-
- for (i = 0; i < nr_regs; i++) {
- ret = imx290_write_reg(imx290, address_low + i,
- (u8)(value >> (i * 8)));
- if (ret) {
- dev_err(imx290->dev, "Error writing buffered registers\n");
- return ret;
- }
- }
-
- ret = imx290_write_reg(imx290, IMX290_REGHOLD, 0x00);
- if (ret) {
- dev_err(imx290->dev, "Error setting hold register\n");
- return ret;
- }
-
- return ret;
-}
-
static int imx290_set_gain(struct imx290 *imx290, u32 value)
{
int ret;
- ret = imx290_write_buffered_reg(imx290, IMX290_GAIN, 1, value);
+ ret = imx290_write_reg(imx290, IMX290_GAIN, value);
if (ret)
dev_err(imx290->dev, "Unable to write gain\n");
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 06/20] media: i2c: imx290: Drop regmap cache
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (4 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 05/20] media: i2c: imx290: Drop imx290_write_buffered_reg() Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 07/20] media: i2c: imx290: Specify HMAX values in decimal Laurent Pinchart
` (13 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Only two registers are ever read, and once only. There's no need to
cache values.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/media/i2c/imx290.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 5646f1704a1e..e1f91b4275c3 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -97,7 +97,6 @@ static const struct imx290_pixfmt imx290_formats[] = {
static const struct regmap_config imx290_regmap_config = {
.reg_bits = 16,
.val_bits = 8,
- .cache_type = REGCACHE_RBTREE,
};
static const char * const imx290_test_pattern_menu[] = {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 07/20] media: i2c: imx290: Specify HMAX values in decimal
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (5 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 06/20] media: i2c: imx290: Drop regmap cache Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 08/20] media: i2c: imx290: Support variable-sized registers Laurent Pinchart
` (12 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
The HMAX value specifies the total line length in pixels. It's thus more
readable in decimal than hexadecimal. Fix it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/i2c/imx290.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index e1f91b4275c3..711282126c34 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -307,7 +307,7 @@ static const struct imx290_mode imx290_modes_2lanes[] = {
{
.width = 1920,
.height = 1080,
- .hmax = 0x1130,
+ .hmax = 4400,
.link_freq_index = FREQ_INDEX_1080P,
.data = imx290_1080p_settings,
.data_size = ARRAY_SIZE(imx290_1080p_settings),
@@ -315,7 +315,7 @@ static const struct imx290_mode imx290_modes_2lanes[] = {
{
.width = 1280,
.height = 720,
- .hmax = 0x19c8,
+ .hmax = 6600,
.link_freq_index = FREQ_INDEX_720P,
.data = imx290_720p_settings,
.data_size = ARRAY_SIZE(imx290_720p_settings),
@@ -326,7 +326,7 @@ static const struct imx290_mode imx290_modes_4lanes[] = {
{
.width = 1920,
.height = 1080,
- .hmax = 0x0898,
+ .hmax = 2200,
.link_freq_index = FREQ_INDEX_1080P,
.data = imx290_1080p_settings,
.data_size = ARRAY_SIZE(imx290_1080p_settings),
@@ -334,7 +334,7 @@ static const struct imx290_mode imx290_modes_4lanes[] = {
{
.width = 1280,
.height = 720,
- .hmax = 0x0ce4,
+ .hmax = 3300,
.link_freq_index = FREQ_INDEX_720P,
.data = imx290_720p_settings,
.data_size = ARRAY_SIZE(imx290_720p_settings),
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 08/20] media: i2c: imx290: Support variable-sized registers
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (6 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 07/20] media: i2c: imx290: Specify HMAX values in decimal Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 09/20] media: i2c: imx290: Correct register sizes Laurent Pinchart
` (11 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
The IMX290 has registers of different sizes. To simplify the code,
handle this in the read/write functions instead of in the callers by
encoding the register size in the symbolic name macros. All registers
are defined as 8-bit for now, a subsequent change will move to larger
registers where applicable.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:
- Use IMX290_REG_SIZE_SHIFT in IMX290_REG_*BIT() macros
- Use unsigned integers in IMX290_REG_*BIT() macros
---
drivers/media/i2c/imx290.c | 352 +++++++++++++++++++------------------
1 file changed, 180 insertions(+), 172 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 711282126c34..6e4662caec62 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -22,22 +22,28 @@
#include <media/v4l2-fwnode.h>
#include <media/v4l2-subdev.h>
-#define IMX290_STANDBY 0x3000
-#define IMX290_REGHOLD 0x3001
-#define IMX290_XMSTA 0x3002
-#define IMX290_FR_FDG_SEL 0x3009
-#define IMX290_BLKLEVEL_LOW 0x300a
-#define IMX290_BLKLEVEL_HIGH 0x300b
-#define IMX290_GAIN 0x3014
-#define IMX290_HMAX_LOW 0x301c
-#define IMX290_HMAX_HIGH 0x301d
-#define IMX290_PGCTRL 0x308c
-#define IMX290_PHY_LANE_NUM 0x3407
-#define IMX290_CSI_LANE_MODE 0x3443
+#define IMX290_REG_SIZE_SHIFT 16
+#define IMX290_REG_ADDR_MASK 0xffff
+#define IMX290_REG_8BIT(n) ((1U << IMX290_REG_SIZE_SHIFT) | (n))
+#define IMX290_REG_16BIT(n) ((2U << IMX290_REG_SIZE_SHIFT) | (n))
+#define IMX290_REG_24BIT(n) ((3U << IMX290_REG_SIZE_SHIFT) | (n))
-#define IMX290_PGCTRL_REGEN BIT(0)
-#define IMX290_PGCTRL_THRU BIT(1)
-#define IMX290_PGCTRL_MODE(n) ((n) << 4)
+#define IMX290_STANDBY IMX290_REG_8BIT(0x3000)
+#define IMX290_REGHOLD IMX290_REG_8BIT(0x3001)
+#define IMX290_XMSTA IMX290_REG_8BIT(0x3002)
+#define IMX290_FR_FDG_SEL IMX290_REG_8BIT(0x3009)
+#define IMX290_BLKLEVEL_LOW IMX290_REG_8BIT(0x300a)
+#define IMX290_BLKLEVEL_HIGH IMX290_REG_8BIT(0x300b)
+#define IMX290_GAIN IMX290_REG_8BIT(0x3014)
+#define IMX290_HMAX_LOW IMX290_REG_8BIT(0x301c)
+#define IMX290_HMAX_HIGH IMX290_REG_8BIT(0x301d)
+#define IMX290_PGCTRL IMX290_REG_8BIT(0x308c)
+#define IMX290_PHY_LANE_NUM IMX290_REG_8BIT(0x3407)
+#define IMX290_CSI_LANE_MODE IMX290_REG_8BIT(0x3443)
+
+#define IMX290_PGCTRL_REGEN BIT(0)
+#define IMX290_PGCTRL_THRU BIT(1)
+#define IMX290_PGCTRL_MODE(n) ((n) << 4)
static const char * const imx290_supply_name[] = {
"vdda",
@@ -48,7 +54,7 @@ static const char * const imx290_supply_name[] = {
#define IMX290_NUM_SUPPLIES ARRAY_SIZE(imx290_supply_name)
struct imx290_regval {
- u16 reg;
+ u32 reg;
u8 val;
};
@@ -111,163 +117,163 @@ static const char * const imx290_test_pattern_menu[] = {
};
static const struct imx290_regval imx290_global_init_settings[] = {
- { 0x3007, 0x00 },
- { 0x3018, 0x65 },
- { 0x3019, 0x04 },
- { 0x301a, 0x00 },
- { 0x3444, 0x20 },
- { 0x3445, 0x25 },
- { 0x303a, 0x0c },
- { 0x3040, 0x00 },
- { 0x3041, 0x00 },
- { 0x303c, 0x00 },
- { 0x303d, 0x00 },
- { 0x3042, 0x9c },
- { 0x3043, 0x07 },
- { 0x303e, 0x49 },
- { 0x303f, 0x04 },
- { 0x304b, 0x0a },
- { 0x300f, 0x00 },
- { 0x3010, 0x21 },
- { 0x3012, 0x64 },
- { 0x3016, 0x09 },
- { 0x3070, 0x02 },
- { 0x3071, 0x11 },
- { 0x309b, 0x10 },
- { 0x309c, 0x22 },
- { 0x30a2, 0x02 },
- { 0x30a6, 0x20 },
- { 0x30a8, 0x20 },
- { 0x30aa, 0x20 },
- { 0x30ac, 0x20 },
- { 0x30b0, 0x43 },
- { 0x3119, 0x9e },
- { 0x311c, 0x1e },
- { 0x311e, 0x08 },
- { 0x3128, 0x05 },
- { 0x313d, 0x83 },
- { 0x3150, 0x03 },
- { 0x317e, 0x00 },
- { 0x32b8, 0x50 },
- { 0x32b9, 0x10 },
- { 0x32ba, 0x00 },
- { 0x32bb, 0x04 },
- { 0x32c8, 0x50 },
- { 0x32c9, 0x10 },
- { 0x32ca, 0x00 },
- { 0x32cb, 0x04 },
- { 0x332c, 0xd3 },
- { 0x332d, 0x10 },
- { 0x332e, 0x0d },
- { 0x3358, 0x06 },
- { 0x3359, 0xe1 },
- { 0x335a, 0x11 },
- { 0x3360, 0x1e },
- { 0x3361, 0x61 },
- { 0x3362, 0x10 },
- { 0x33b0, 0x50 },
- { 0x33b2, 0x1a },
- { 0x33b3, 0x04 },
+ { IMX290_REG_8BIT(0x3007), 0x00 },
+ { IMX290_REG_8BIT(0x3018), 0x65 },
+ { IMX290_REG_8BIT(0x3019), 0x04 },
+ { IMX290_REG_8BIT(0x301a), 0x00 },
+ { IMX290_REG_8BIT(0x3444), 0x20 },
+ { IMX290_REG_8BIT(0x3445), 0x25 },
+ { IMX290_REG_8BIT(0x303a), 0x0c },
+ { IMX290_REG_8BIT(0x3040), 0x00 },
+ { IMX290_REG_8BIT(0x3041), 0x00 },
+ { IMX290_REG_8BIT(0x303c), 0x00 },
+ { IMX290_REG_8BIT(0x303d), 0x00 },
+ { IMX290_REG_8BIT(0x3042), 0x9c },
+ { IMX290_REG_8BIT(0x3043), 0x07 },
+ { IMX290_REG_8BIT(0x303e), 0x49 },
+ { IMX290_REG_8BIT(0x303f), 0x04 },
+ { IMX290_REG_8BIT(0x304b), 0x0a },
+ { IMX290_REG_8BIT(0x300f), 0x00 },
+ { IMX290_REG_8BIT(0x3010), 0x21 },
+ { IMX290_REG_8BIT(0x3012), 0x64 },
+ { IMX290_REG_8BIT(0x3016), 0x09 },
+ { IMX290_REG_8BIT(0x3070), 0x02 },
+ { IMX290_REG_8BIT(0x3071), 0x11 },
+ { IMX290_REG_8BIT(0x309b), 0x10 },
+ { IMX290_REG_8BIT(0x309c), 0x22 },
+ { IMX290_REG_8BIT(0x30a2), 0x02 },
+ { IMX290_REG_8BIT(0x30a6), 0x20 },
+ { IMX290_REG_8BIT(0x30a8), 0x20 },
+ { IMX290_REG_8BIT(0x30aa), 0x20 },
+ { IMX290_REG_8BIT(0x30ac), 0x20 },
+ { IMX290_REG_8BIT(0x30b0), 0x43 },
+ { IMX290_REG_8BIT(0x3119), 0x9e },
+ { IMX290_REG_8BIT(0x311c), 0x1e },
+ { IMX290_REG_8BIT(0x311e), 0x08 },
+ { IMX290_REG_8BIT(0x3128), 0x05 },
+ { IMX290_REG_8BIT(0x313d), 0x83 },
+ { IMX290_REG_8BIT(0x3150), 0x03 },
+ { IMX290_REG_8BIT(0x317e), 0x00 },
+ { IMX290_REG_8BIT(0x32b8), 0x50 },
+ { IMX290_REG_8BIT(0x32b9), 0x10 },
+ { IMX290_REG_8BIT(0x32ba), 0x00 },
+ { IMX290_REG_8BIT(0x32bb), 0x04 },
+ { IMX290_REG_8BIT(0x32c8), 0x50 },
+ { IMX290_REG_8BIT(0x32c9), 0x10 },
+ { IMX290_REG_8BIT(0x32ca), 0x00 },
+ { IMX290_REG_8BIT(0x32cb), 0x04 },
+ { IMX290_REG_8BIT(0x332c), 0xd3 },
+ { IMX290_REG_8BIT(0x332d), 0x10 },
+ { IMX290_REG_8BIT(0x332e), 0x0d },
+ { IMX290_REG_8BIT(0x3358), 0x06 },
+ { IMX290_REG_8BIT(0x3359), 0xe1 },
+ { IMX290_REG_8BIT(0x335a), 0x11 },
+ { IMX290_REG_8BIT(0x3360), 0x1e },
+ { IMX290_REG_8BIT(0x3361), 0x61 },
+ { IMX290_REG_8BIT(0x3362), 0x10 },
+ { IMX290_REG_8BIT(0x33b0), 0x50 },
+ { IMX290_REG_8BIT(0x33b2), 0x1a },
+ { IMX290_REG_8BIT(0x33b3), 0x04 },
};
static const struct imx290_regval imx290_1080p_settings[] = {
/* mode settings */
- { 0x3007, 0x00 },
- { 0x303a, 0x0c },
- { 0x3414, 0x0a },
- { 0x3472, 0x80 },
- { 0x3473, 0x07 },
- { 0x3418, 0x38 },
- { 0x3419, 0x04 },
- { 0x3012, 0x64 },
- { 0x3013, 0x00 },
- { 0x305c, 0x18 },
- { 0x305d, 0x03 },
- { 0x305e, 0x20 },
- { 0x305f, 0x01 },
- { 0x315e, 0x1a },
- { 0x3164, 0x1a },
- { 0x3480, 0x49 },
+ { IMX290_REG_8BIT(0x3007), 0x00 },
+ { IMX290_REG_8BIT(0x303a), 0x0c },
+ { IMX290_REG_8BIT(0x3414), 0x0a },
+ { IMX290_REG_8BIT(0x3472), 0x80 },
+ { IMX290_REG_8BIT(0x3473), 0x07 },
+ { IMX290_REG_8BIT(0x3418), 0x38 },
+ { IMX290_REG_8BIT(0x3419), 0x04 },
+ { IMX290_REG_8BIT(0x3012), 0x64 },
+ { IMX290_REG_8BIT(0x3013), 0x00 },
+ { IMX290_REG_8BIT(0x305c), 0x18 },
+ { IMX290_REG_8BIT(0x305d), 0x03 },
+ { IMX290_REG_8BIT(0x305e), 0x20 },
+ { IMX290_REG_8BIT(0x305f), 0x01 },
+ { IMX290_REG_8BIT(0x315e), 0x1a },
+ { IMX290_REG_8BIT(0x3164), 0x1a },
+ { IMX290_REG_8BIT(0x3480), 0x49 },
/* data rate settings */
- { 0x3405, 0x10 },
- { 0x3446, 0x57 },
- { 0x3447, 0x00 },
- { 0x3448, 0x37 },
- { 0x3449, 0x00 },
- { 0x344a, 0x1f },
- { 0x344b, 0x00 },
- { 0x344c, 0x1f },
- { 0x344d, 0x00 },
- { 0x344e, 0x1f },
- { 0x344f, 0x00 },
- { 0x3450, 0x77 },
- { 0x3451, 0x00 },
- { 0x3452, 0x1f },
- { 0x3453, 0x00 },
- { 0x3454, 0x17 },
- { 0x3455, 0x00 },
+ { IMX290_REG_8BIT(0x3405), 0x10 },
+ { IMX290_REG_8BIT(0x3446), 0x57 },
+ { IMX290_REG_8BIT(0x3447), 0x00 },
+ { IMX290_REG_8BIT(0x3448), 0x37 },
+ { IMX290_REG_8BIT(0x3449), 0x00 },
+ { IMX290_REG_8BIT(0x344a), 0x1f },
+ { IMX290_REG_8BIT(0x344b), 0x00 },
+ { IMX290_REG_8BIT(0x344c), 0x1f },
+ { IMX290_REG_8BIT(0x344d), 0x00 },
+ { IMX290_REG_8BIT(0x344e), 0x1f },
+ { IMX290_REG_8BIT(0x344f), 0x00 },
+ { IMX290_REG_8BIT(0x3450), 0x77 },
+ { IMX290_REG_8BIT(0x3451), 0x00 },
+ { IMX290_REG_8BIT(0x3452), 0x1f },
+ { IMX290_REG_8BIT(0x3453), 0x00 },
+ { IMX290_REG_8BIT(0x3454), 0x17 },
+ { IMX290_REG_8BIT(0x3455), 0x00 },
};
static const struct imx290_regval imx290_720p_settings[] = {
/* mode settings */
- { 0x3007, 0x10 },
- { 0x303a, 0x06 },
- { 0x3414, 0x04 },
- { 0x3472, 0x00 },
- { 0x3473, 0x05 },
- { 0x3418, 0xd0 },
- { 0x3419, 0x02 },
- { 0x3012, 0x64 },
- { 0x3013, 0x00 },
- { 0x305c, 0x20 },
- { 0x305d, 0x00 },
- { 0x305e, 0x20 },
- { 0x305f, 0x01 },
- { 0x315e, 0x1a },
- { 0x3164, 0x1a },
- { 0x3480, 0x49 },
+ { IMX290_REG_8BIT(0x3007), 0x10 },
+ { IMX290_REG_8BIT(0x303a), 0x06 },
+ { IMX290_REG_8BIT(0x3414), 0x04 },
+ { IMX290_REG_8BIT(0x3472), 0x00 },
+ { IMX290_REG_8BIT(0x3473), 0x05 },
+ { IMX290_REG_8BIT(0x3418), 0xd0 },
+ { IMX290_REG_8BIT(0x3419), 0x02 },
+ { IMX290_REG_8BIT(0x3012), 0x64 },
+ { IMX290_REG_8BIT(0x3013), 0x00 },
+ { IMX290_REG_8BIT(0x305c), 0x20 },
+ { IMX290_REG_8BIT(0x305d), 0x00 },
+ { IMX290_REG_8BIT(0x305e), 0x20 },
+ { IMX290_REG_8BIT(0x305f), 0x01 },
+ { IMX290_REG_8BIT(0x315e), 0x1a },
+ { IMX290_REG_8BIT(0x3164), 0x1a },
+ { IMX290_REG_8BIT(0x3480), 0x49 },
/* data rate settings */
- { 0x3405, 0x10 },
- { 0x3446, 0x4f },
- { 0x3447, 0x00 },
- { 0x3448, 0x2f },
- { 0x3449, 0x00 },
- { 0x344a, 0x17 },
- { 0x344b, 0x00 },
- { 0x344c, 0x17 },
- { 0x344d, 0x00 },
- { 0x344e, 0x17 },
- { 0x344f, 0x00 },
- { 0x3450, 0x57 },
- { 0x3451, 0x00 },
- { 0x3452, 0x17 },
- { 0x3453, 0x00 },
- { 0x3454, 0x17 },
- { 0x3455, 0x00 },
+ { IMX290_REG_8BIT(0x3405), 0x10 },
+ { IMX290_REG_8BIT(0x3446), 0x4f },
+ { IMX290_REG_8BIT(0x3447), 0x00 },
+ { IMX290_REG_8BIT(0x3448), 0x2f },
+ { IMX290_REG_8BIT(0x3449), 0x00 },
+ { IMX290_REG_8BIT(0x344a), 0x17 },
+ { IMX290_REG_8BIT(0x344b), 0x00 },
+ { IMX290_REG_8BIT(0x344c), 0x17 },
+ { IMX290_REG_8BIT(0x344d), 0x00 },
+ { IMX290_REG_8BIT(0x344e), 0x17 },
+ { IMX290_REG_8BIT(0x344f), 0x00 },
+ { IMX290_REG_8BIT(0x3450), 0x57 },
+ { IMX290_REG_8BIT(0x3451), 0x00 },
+ { IMX290_REG_8BIT(0x3452), 0x17 },
+ { IMX290_REG_8BIT(0x3453), 0x00 },
+ { IMX290_REG_8BIT(0x3454), 0x17 },
+ { IMX290_REG_8BIT(0x3455), 0x00 },
};
static const struct imx290_regval imx290_10bit_settings[] = {
- { 0x3005, 0x00},
- { 0x3046, 0x00},
- { 0x3129, 0x1d},
- { 0x317c, 0x12},
- { 0x31ec, 0x37},
- { 0x3441, 0x0a},
- { 0x3442, 0x0a},
- { 0x300a, 0x3c},
- { 0x300b, 0x00},
+ { IMX290_REG_8BIT(0x3005), 0x00},
+ { IMX290_REG_8BIT(0x3046), 0x00},
+ { IMX290_REG_8BIT(0x3129), 0x1d},
+ { IMX290_REG_8BIT(0x317c), 0x12},
+ { IMX290_REG_8BIT(0x31ec), 0x37},
+ { IMX290_REG_8BIT(0x3441), 0x0a},
+ { IMX290_REG_8BIT(0x3442), 0x0a},
+ { IMX290_REG_8BIT(0x300a), 0x3c},
+ { IMX290_REG_8BIT(0x300b), 0x00},
};
static const struct imx290_regval imx290_12bit_settings[] = {
- { 0x3005, 0x01 },
- { 0x3046, 0x01 },
- { 0x3129, 0x00 },
- { 0x317c, 0x00 },
- { 0x31ec, 0x0e },
- { 0x3441, 0x0c },
- { 0x3442, 0x0c },
- { 0x300a, 0xf0 },
- { 0x300b, 0x00 },
+ { IMX290_REG_8BIT(0x3005), 0x01 },
+ { IMX290_REG_8BIT(0x3046), 0x01 },
+ { IMX290_REG_8BIT(0x3129), 0x00 },
+ { IMX290_REG_8BIT(0x317c), 0x00 },
+ { IMX290_REG_8BIT(0x31ec), 0x0e },
+ { IMX290_REG_8BIT(0x3441), 0x0c },
+ { IMX290_REG_8BIT(0x3442), 0x0c },
+ { IMX290_REG_8BIT(0x300a), 0xf0 },
+ { IMX290_REG_8BIT(0x300b), 0x00 },
};
/* supported link frequencies */
@@ -362,33 +368,35 @@ static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
return container_of(_sd, struct imx290, sd);
}
-static inline int __always_unused imx290_read_reg(struct imx290 *imx290, u16 addr, u8 *value)
+static int __always_unused imx290_read_reg(struct imx290 *imx290, u32 addr, u32 *value)
{
- unsigned int regval;
+ u8 data[3] = { 0, 0, 0 };
int ret;
- ret = regmap_read(imx290->regmap, addr, ®val);
- if (ret) {
- dev_err(imx290->dev, "Failed to read register 0x%04x: %d\n",
- addr, ret);
+ ret = regmap_raw_read(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
+ data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
+ if (ret < 0) {
+ dev_err(imx290->dev, "%u-bit read from 0x%04x failed: %d\n",
+ ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
+ addr & IMX290_REG_ADDR_MASK, ret);
return ret;
}
- *value = regval & 0xff;
-
+ *value = (data[2] << 16) | (data[1] << 8) | data[0];
return 0;
}
-static int imx290_write_reg(struct imx290 *imx290, u16 addr, u8 value)
+static int imx290_write_reg(struct imx290 *imx290, u32 addr, u32 value)
{
+ u8 data[3] = { value & 0xff, (value >> 8) & 0xff, value >> 16 };
int ret;
- ret = regmap_write(imx290->regmap, addr, value);
- if (ret) {
- dev_err(imx290->dev, "Failed to write register 0x%04x: %d\n",
- addr, ret);
- return ret;
- }
+ ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
+ data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
+ if (ret < 0)
+ dev_err(imx290->dev, "%u-bit write to 0x%04x failed: %d\n",
+ ((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
+ addr & IMX290_REG_ADDR_MASK, ret);
return ret;
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 09/20] media: i2c: imx290: Correct register sizes
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (7 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 08/20] media: i2c: imx290: Support variable-sized registers Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 10/20] media: i2c: imx290: Simplify error handling when writing registers Laurent Pinchart
` (10 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Define registers with the appropriate size, using the variable-size
register access mechanism that has just been introduced. This simplifies
the code.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/i2c/imx290.c | 39 +++++++++-----------------------------
1 file changed, 9 insertions(+), 30 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 6e4662caec62..cfcfa04e83e1 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -32,12 +32,11 @@
#define IMX290_REGHOLD IMX290_REG_8BIT(0x3001)
#define IMX290_XMSTA IMX290_REG_8BIT(0x3002)
#define IMX290_FR_FDG_SEL IMX290_REG_8BIT(0x3009)
-#define IMX290_BLKLEVEL_LOW IMX290_REG_8BIT(0x300a)
-#define IMX290_BLKLEVEL_HIGH IMX290_REG_8BIT(0x300b)
+#define IMX290_BLKLEVEL IMX290_REG_16BIT(0x300a)
#define IMX290_GAIN IMX290_REG_8BIT(0x3014)
-#define IMX290_HMAX_LOW IMX290_REG_8BIT(0x301c)
-#define IMX290_HMAX_HIGH IMX290_REG_8BIT(0x301d)
+#define IMX290_HMAX IMX290_REG_16BIT(0x301c)
#define IMX290_PGCTRL IMX290_REG_8BIT(0x308c)
+#define IMX290_CHIP_ID IMX290_REG_16BIT(0x319a)
#define IMX290_PHY_LANE_NUM IMX290_REG_8BIT(0x3407)
#define IMX290_CSI_LANE_MODE IMX290_REG_8BIT(0x3443)
@@ -461,8 +460,7 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
break;
case V4L2_CID_TEST_PATTERN:
if (ctrl->val) {
- imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW, 0x00);
- imx290_write_reg(imx290, IMX290_BLKLEVEL_HIGH, 0x00);
+ imx290_write_reg(imx290, IMX290_BLKLEVEL, 0);
usleep_range(10000, 11000);
imx290_write_reg(imx290, IMX290_PGCTRL,
(u8)(IMX290_PGCTRL_REGEN |
@@ -472,12 +470,11 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
imx290_write_reg(imx290, IMX290_PGCTRL, 0x00);
usleep_range(10000, 11000);
if (imx290->bpp == 10)
- imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW,
+ imx290_write_reg(imx290, IMX290_BLKLEVEL,
0x3c);
else /* 12 bits per pixel */
- imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW,
+ imx290_write_reg(imx290, IMX290_BLKLEVEL,
0xf0);
- imx290_write_reg(imx290, IMX290_BLKLEVEL_HIGH, 0x00);
}
break;
default:
@@ -669,25 +666,6 @@ static int imx290_write_current_format(struct imx290 *imx290)
return 0;
}
-static int imx290_set_hmax(struct imx290 *imx290, u32 val)
-{
- int ret;
-
- ret = imx290_write_reg(imx290, IMX290_HMAX_LOW, (val & 0xff));
- if (ret) {
- dev_err(imx290->dev, "Error setting HMAX register\n");
- return ret;
- }
-
- ret = imx290_write_reg(imx290, IMX290_HMAX_HIGH, ((val >> 8) & 0xff));
- if (ret) {
- dev_err(imx290->dev, "Error setting HMAX register\n");
- return ret;
- }
-
- return 0;
-}
-
/* Start streaming */
static int imx290_start_streaming(struct imx290 *imx290)
{
@@ -716,8 +694,9 @@ static int imx290_start_streaming(struct imx290 *imx290)
dev_err(imx290->dev, "Could not set current mode\n");
return ret;
}
- ret = imx290_set_hmax(imx290, imx290->current_mode->hmax);
- if (ret < 0)
+
+ ret = imx290_write_reg(imx290, IMX290_HMAX, imx290->current_mode->hmax);
+ if (ret)
return ret;
/* Apply customized values from user */
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 10/20] media: i2c: imx290: Simplify error handling when writing registers
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (8 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 09/20] media: i2c: imx290: Correct register sizes Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 11/20] media: i2c: imx290: Define more register macros Laurent Pinchart
` (9 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Error handling for register writes requires checking the error status of
every single write. This makes the code complex, or incorrect when the
checks are omitted. Simplify this by passing a pointer to an error code
to the imx290_write_reg() function, which allows writing multiple
registers in a row and only checking for errors at the end.
While at it, rename imx290_write_reg() to imx290_write() as there's
nothing else than registers to write, and rename imx290_read_reg()
accordingly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/media/i2c/imx290.c | 86 ++++++++++++++------------------------
1 file changed, 32 insertions(+), 54 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index cfcfa04e83e1..0f26da5c2e54 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -367,7 +367,7 @@ static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
return container_of(_sd, struct imx290, sd);
}
-static int __always_unused imx290_read_reg(struct imx290 *imx290, u32 addr, u32 *value)
+static int __always_unused imx290_read(struct imx290 *imx290, u32 addr, u32 *value)
{
u8 data[3] = { 0, 0, 0 };
int ret;
@@ -385,17 +385,23 @@ static int __always_unused imx290_read_reg(struct imx290 *imx290, u32 addr, u32
return 0;
}
-static int imx290_write_reg(struct imx290 *imx290, u32 addr, u32 value)
+static int imx290_write(struct imx290 *imx290, u32 addr, u32 value, int *err)
{
u8 data[3] = { value & 0xff, (value >> 8) & 0xff, value >> 16 };
int ret;
+ if (err && *err)
+ return *err;
+
ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
- if (ret < 0)
+ if (ret < 0) {
dev_err(imx290->dev, "%u-bit write to 0x%04x failed: %d\n",
((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
addr & IMX290_REG_ADDR_MASK, ret);
+ if (err)
+ *err = ret;
+ }
return ret;
}
@@ -408,7 +414,7 @@ static int imx290_set_register_array(struct imx290 *imx290,
int ret;
for (i = 0; i < num_settings; ++i, ++settings) {
- ret = imx290_write_reg(imx290, settings->reg, settings->val);
+ ret = imx290_write(imx290, settings->reg, settings->val, NULL);
if (ret < 0)
return ret;
}
@@ -419,29 +425,16 @@ static int imx290_set_register_array(struct imx290 *imx290,
return 0;
}
-static int imx290_set_gain(struct imx290 *imx290, u32 value)
-{
- int ret;
-
- ret = imx290_write_reg(imx290, IMX290_GAIN, value);
- if (ret)
- dev_err(imx290->dev, "Unable to write gain\n");
-
- return ret;
-}
-
/* Stop streaming */
static int imx290_stop_streaming(struct imx290 *imx290)
{
- int ret;
+ int ret = 0;
- ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x01);
- if (ret < 0)
- return ret;
+ imx290_write(imx290, IMX290_STANDBY, 0x01, &ret);
msleep(30);
- return imx290_write_reg(imx290, IMX290_XMSTA, 0x01);
+ return imx290_write(imx290, IMX290_XMSTA, 0x01, &ret);
}
static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
@@ -456,25 +449,25 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
switch (ctrl->id) {
case V4L2_CID_GAIN:
- ret = imx290_set_gain(imx290, ctrl->val);
+ ret = imx290_write(imx290, IMX290_GAIN, ctrl->val, NULL);
break;
case V4L2_CID_TEST_PATTERN:
if (ctrl->val) {
- imx290_write_reg(imx290, IMX290_BLKLEVEL, 0);
+ imx290_write(imx290, IMX290_BLKLEVEL, 0, &ret);
usleep_range(10000, 11000);
- imx290_write_reg(imx290, IMX290_PGCTRL,
- (u8)(IMX290_PGCTRL_REGEN |
- IMX290_PGCTRL_THRU |
- IMX290_PGCTRL_MODE(ctrl->val)));
+ imx290_write(imx290, IMX290_PGCTRL,
+ (u8)(IMX290_PGCTRL_REGEN |
+ IMX290_PGCTRL_THRU |
+ IMX290_PGCTRL_MODE(ctrl->val)), &ret);
} else {
- imx290_write_reg(imx290, IMX290_PGCTRL, 0x00);
+ imx290_write(imx290, IMX290_PGCTRL, 0x00, &ret);
usleep_range(10000, 11000);
if (imx290->bpp == 10)
- imx290_write_reg(imx290, IMX290_BLKLEVEL,
- 0x3c);
+ imx290_write(imx290, IMX290_BLKLEVEL, 0x3c,
+ &ret);
else /* 12 bits per pixel */
- imx290_write_reg(imx290, IMX290_BLKLEVEL,
- 0xf0);
+ imx290_write(imx290, IMX290_BLKLEVEL, 0xf0,
+ &ret);
}
break;
default:
@@ -695,7 +688,8 @@ static int imx290_start_streaming(struct imx290 *imx290)
return ret;
}
- ret = imx290_write_reg(imx290, IMX290_HMAX, imx290->current_mode->hmax);
+ ret = imx290_write(imx290, IMX290_HMAX, imx290->current_mode->hmax,
+ NULL);
if (ret)
return ret;
@@ -706,14 +700,12 @@ static int imx290_start_streaming(struct imx290 *imx290)
return ret;
}
- ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x00);
- if (ret < 0)
- return ret;
+ imx290_write(imx290, IMX290_STANDBY, 0x00, &ret);
msleep(30);
/* Start streaming */
- return imx290_write_reg(imx290, IMX290_XMSTA, 0x00);
+ return imx290_write(imx290, IMX290_XMSTA, 0x00, &ret);
}
static int imx290_set_stream(struct v4l2_subdev *sd, int enable)
@@ -772,27 +764,13 @@ static int imx290_set_data_lanes(struct imx290 *imx290)
* validated in probe itself
*/
dev_err(imx290->dev, "Lane configuration not supported\n");
- ret = -EINVAL;
- goto exit;
+ return -EINVAL;
}
- ret = imx290_write_reg(imx290, IMX290_PHY_LANE_NUM, laneval);
- if (ret) {
- dev_err(imx290->dev, "Error setting Physical Lane number register\n");
- goto exit;
- }
-
- ret = imx290_write_reg(imx290, IMX290_CSI_LANE_MODE, laneval);
- if (ret) {
- dev_err(imx290->dev, "Error setting CSI Lane mode register\n");
- goto exit;
- }
-
- ret = imx290_write_reg(imx290, IMX290_FR_FDG_SEL, frsel);
- if (ret)
- dev_err(imx290->dev, "Error setting FR/FDG SEL register\n");
+ imx290_write(imx290, IMX290_PHY_LANE_NUM, laneval, &ret);
+ imx290_write(imx290, IMX290_CSI_LANE_MODE, laneval, &ret);
+ imx290_write(imx290, IMX290_FR_FDG_SEL, frsel, &ret);
-exit:
return ret;
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 11/20] media: i2c: imx290: Define more register macros
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (9 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 10/20] media: i2c: imx290: Simplify error handling when writing registers Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-17 6:07 ` Alexander Stein
2022-10-17 8:35 ` [PATCH v2.1 " Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 12/20] media: i2c: imx290: Add exposure time control Laurent Pinchart
` (8 subsequent siblings)
19 siblings, 2 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Define macros for all registers programmed by the driver for which
documentation is available to increase readability. This starts making
use of 16-bit registers in the register arrays, so the value field has
to be increased to 32 bits.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/i2c/imx290.c | 219 +++++++++++++++++++++----------------
1 file changed, 124 insertions(+), 95 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 0f26da5c2e54..93fd043669dc 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -31,14 +31,73 @@
#define IMX290_STANDBY IMX290_REG_8BIT(0x3000)
#define IMX290_REGHOLD IMX290_REG_8BIT(0x3001)
#define IMX290_XMSTA IMX290_REG_8BIT(0x3002)
+#define IMX290_ADBIT IMX290_REG_8BIT(0x3005)
+#define IMX290_ADBIT_10BIT (0 << 0)
+#define IMX290_ADBIT_12BIT (1 << 0)
+#define IMX290_CTRL_07 IMX290_REG_8BIT(0x3007)
+#define IMX290_VREVERSE BIT(0)
+#define IMX290_HREVERSE BIT(1)
+#define IMX290_WINMODE_1080P (0 << 4)
+#define IMX290_WINMODE_720P (1 << 4)
+#define IMX290_WINMODE_CROP (4 << 4)
#define IMX290_FR_FDG_SEL IMX290_REG_8BIT(0x3009)
#define IMX290_BLKLEVEL IMX290_REG_16BIT(0x300a)
#define IMX290_GAIN IMX290_REG_8BIT(0x3014)
+#define IMX290_VMAX IMX290_REG_24BIT(0x3018)
#define IMX290_HMAX IMX290_REG_16BIT(0x301c)
+#define IMX290_SHS1 IMX290_REG_24BIT(0x3020)
+#define IMX290_WINWV_OB IMX290_REG_8BIT(0x303a)
+#define IMX290_WINPV IMX290_REG_16BIT(0x303c)
+#define IMX290_WINWV IMX290_REG_16BIT(0x303e)
+#define IMX290_WINPH IMX290_REG_16BIT(0x3040)
+#define IMX290_WINWH IMX290_REG_16BIT(0x3042)
+#define IMX290_OUT_CTRL IMX290_REG_8BIT(0x3046)
+#define IMX290_ODBIT_10BIT (0 << 0)
+#define IMX290_ODBIT_12BIT (1 << 0)
+#define IMX290_OPORTSEL_PARALLEL (0x0 << 4)
+#define IMX290_OPORTSEL_LVDS_2CH (0xd << 4)
+#define IMX290_OPORTSEL_LVDS_4CH (0xe << 4)
+#define IMX290_OPORTSEL_LVDS_8CH (0xf << 4)
+#define IMX290_XSOUTSEL IMX290_REG_8BIT(0x304b)
+#define IMX290_XSOUTSEL_XVSOUTSEL_HIGH (0 << 0)
+#define IMX290_XSOUTSEL_XVSOUTSEL_VSYNC (2 << 0)
+#define IMX290_XSOUTSEL_XHSOUTSEL_HIGH (0 << 2)
+#define IMX290_XSOUTSEL_XHSOUTSEL_HSYNC (2 << 2)
+#define IMX290_INCKSEL1 IMX290_REG_8BIT(0x305c)
+#define IMX290_INCKSEL2 IMX290_REG_8BIT(0x305d)
+#define IMX290_INCKSEL3 IMX290_REG_8BIT(0x305e)
+#define IMX290_INCKSEL4 IMX290_REG_8BIT(0x305f)
#define IMX290_PGCTRL IMX290_REG_8BIT(0x308c)
+#define IMX290_ADBIT1 IMX290_REG_8BIT(0x3129)
+#define IMX290_ADBIT1_10BIT 0x1d
+#define IMX290_ADBIT1_12BIT 0x00
+#define IMX290_INCKSEL5 IMX290_REG_8BIT(0x315e)
+#define IMX290_INCKSEL6 IMX290_REG_8BIT(0x3164)
+#define IMX290_ADBIT2 IMX290_REG_8BIT(0x317c)
+#define IMX290_ADBIT2_10BIT 0x12
+#define IMX290_ADBIT2_12BIT 0x00
#define IMX290_CHIP_ID IMX290_REG_16BIT(0x319a)
+#define IMX290_ADBIT3 IMX290_REG_16BIT(0x31ec)
+#define IMX290_ADBIT3_10BIT 0x37
+#define IMX290_ADBIT3_12BIT 0x0e
+#define IMX290_REPETITION IMX290_REG_8BIT(0x3405)
#define IMX290_PHY_LANE_NUM IMX290_REG_8BIT(0x3407)
+#define IMX290_OPB_SIZE_V IMX290_REG_8BIT(0x3414)
+#define IMX290_Y_OUT_SIZE IMX290_REG_16BIT(0x3418)
+#define IMX290_CSI_DT_FMT IMX290_REG_16BIT(0x3441)
+#define IMX290_CSI_DT_FMT_RAW10 0x0a0a
+#define IMX290_CSI_DT_FMT_RAW12 0x0c0c
#define IMX290_CSI_LANE_MODE IMX290_REG_8BIT(0x3443)
+#define IMX290_EXTCK_FREQ IMX290_REG_16BIT(0x3444)
+#define IMX290_TCLKPOST IMX290_REG_16BIT(0x3446)
+#define IMX290_THSZERO IMX290_REG_16BIT(0x3448)
+#define IMX290_THSPREPARE IMX290_REG_16BIT(0x344a)
+#define IMX290_TCLKTRAIL IMX290_REG_16BIT(0x344c)
+#define IMX290_THSTRAIL IMX290_REG_16BIT(0x344e)
+#define IMX290_TCLKZERO IMX290_REG_16BIT(0x3450)
+#define IMX290_TCLKPREPARE IMX290_REG_16BIT(0x3452)
+#define IMX290_TLPX IMX290_REG_16BIT(0x3454)
+#define IMX290_X_OUT_SIZE IMX290_REG_16BIT(0x3472)
#define IMX290_PGCTRL_REGEN BIT(0)
#define IMX290_PGCTRL_THRU BIT(1)
@@ -54,7 +113,7 @@ static const char * const imx290_supply_name[] = {
struct imx290_regval {
u32 reg;
- u8 val;
+ u32 val;
};
struct imx290_mode {
@@ -116,22 +175,16 @@ static const char * const imx290_test_pattern_menu[] = {
};
static const struct imx290_regval imx290_global_init_settings[] = {
- { IMX290_REG_8BIT(0x3007), 0x00 },
- { IMX290_REG_8BIT(0x3018), 0x65 },
- { IMX290_REG_8BIT(0x3019), 0x04 },
- { IMX290_REG_8BIT(0x301a), 0x00 },
- { IMX290_REG_8BIT(0x3444), 0x20 },
- { IMX290_REG_8BIT(0x3445), 0x25 },
- { IMX290_REG_8BIT(0x303a), 0x0c },
- { IMX290_REG_8BIT(0x3040), 0x00 },
- { IMX290_REG_8BIT(0x3041), 0x00 },
- { IMX290_REG_8BIT(0x303c), 0x00 },
- { IMX290_REG_8BIT(0x303d), 0x00 },
- { IMX290_REG_8BIT(0x3042), 0x9c },
- { IMX290_REG_8BIT(0x3043), 0x07 },
- { IMX290_REG_8BIT(0x303e), 0x49 },
- { IMX290_REG_8BIT(0x303f), 0x04 },
- { IMX290_REG_8BIT(0x304b), 0x0a },
+ { IMX290_CTRL_07, IMX290_WINMODE_1080P },
+ { IMX290_VMAX, 1125 },
+ { IMX290_EXTCK_FREQ, 0x2520 },
+ { IMX290_WINWV_OB, 12 },
+ { IMX290_WINPH, 0 },
+ { IMX290_WINPV, 0 },
+ { IMX290_WINWH, 1948 },
+ { IMX290_WINWV, 1097 },
+ { IMX290_XSOUTSEL, IMX290_XSOUTSEL_XVSOUTSEL_VSYNC |
+ IMX290_XSOUTSEL_XHSOUTSEL_HSYNC },
{ IMX290_REG_8BIT(0x300f), 0x00 },
{ IMX290_REG_8BIT(0x3010), 0x21 },
{ IMX290_REG_8BIT(0x3012), 0x64 },
@@ -177,102 +230,78 @@ static const struct imx290_regval imx290_global_init_settings[] = {
static const struct imx290_regval imx290_1080p_settings[] = {
/* mode settings */
- { IMX290_REG_8BIT(0x3007), 0x00 },
- { IMX290_REG_8BIT(0x303a), 0x0c },
- { IMX290_REG_8BIT(0x3414), 0x0a },
- { IMX290_REG_8BIT(0x3472), 0x80 },
- { IMX290_REG_8BIT(0x3473), 0x07 },
- { IMX290_REG_8BIT(0x3418), 0x38 },
- { IMX290_REG_8BIT(0x3419), 0x04 },
+ { IMX290_CTRL_07, IMX290_WINMODE_1080P },
+ { IMX290_WINWV_OB, 12 },
+ { IMX290_OPB_SIZE_V, 10 },
+ { IMX290_X_OUT_SIZE, 1920 },
+ { IMX290_Y_OUT_SIZE, 1080 },
{ IMX290_REG_8BIT(0x3012), 0x64 },
{ IMX290_REG_8BIT(0x3013), 0x00 },
- { IMX290_REG_8BIT(0x305c), 0x18 },
- { IMX290_REG_8BIT(0x305d), 0x03 },
- { IMX290_REG_8BIT(0x305e), 0x20 },
- { IMX290_REG_8BIT(0x305f), 0x01 },
- { IMX290_REG_8BIT(0x315e), 0x1a },
- { IMX290_REG_8BIT(0x3164), 0x1a },
+ { IMX290_INCKSEL1, 0x18 },
+ { IMX290_INCKSEL2, 0x03 },
+ { IMX290_INCKSEL3, 0x20 },
+ { IMX290_INCKSEL4, 0x01 },
+ { IMX290_INCKSEL5, 0x1a },
+ { IMX290_INCKSEL6, 0x1a },
{ IMX290_REG_8BIT(0x3480), 0x49 },
/* data rate settings */
- { IMX290_REG_8BIT(0x3405), 0x10 },
- { IMX290_REG_8BIT(0x3446), 0x57 },
- { IMX290_REG_8BIT(0x3447), 0x00 },
- { IMX290_REG_8BIT(0x3448), 0x37 },
- { IMX290_REG_8BIT(0x3449), 0x00 },
- { IMX290_REG_8BIT(0x344a), 0x1f },
- { IMX290_REG_8BIT(0x344b), 0x00 },
- { IMX290_REG_8BIT(0x344c), 0x1f },
- { IMX290_REG_8BIT(0x344d), 0x00 },
- { IMX290_REG_8BIT(0x344e), 0x1f },
- { IMX290_REG_8BIT(0x344f), 0x00 },
- { IMX290_REG_8BIT(0x3450), 0x77 },
- { IMX290_REG_8BIT(0x3451), 0x00 },
- { IMX290_REG_8BIT(0x3452), 0x1f },
- { IMX290_REG_8BIT(0x3453), 0x00 },
- { IMX290_REG_8BIT(0x3454), 0x17 },
- { IMX290_REG_8BIT(0x3455), 0x00 },
+ { IMX290_REPETITION, 0x10 },
+ { IMX290_TCLKPOST, 87 },
+ { IMX290_THSZERO, 55 },
+ { IMX290_THSPREPARE, 31 },
+ { IMX290_TCLKTRAIL, 31 },
+ { IMX290_THSTRAIL, 31 },
+ { IMX290_TCLKZERO, 119 },
+ { IMX290_TCLKPREPARE, 31 },
+ { IMX290_TLPX, 23 },
};
static const struct imx290_regval imx290_720p_settings[] = {
/* mode settings */
- { IMX290_REG_8BIT(0x3007), 0x10 },
- { IMX290_REG_8BIT(0x303a), 0x06 },
- { IMX290_REG_8BIT(0x3414), 0x04 },
- { IMX290_REG_8BIT(0x3472), 0x00 },
- { IMX290_REG_8BIT(0x3473), 0x05 },
- { IMX290_REG_8BIT(0x3418), 0xd0 },
- { IMX290_REG_8BIT(0x3419), 0x02 },
+ { IMX290_CTRL_07, IMX290_WINMODE_720P },
+ { IMX290_WINWV_OB, 6 },
+ { IMX290_OPB_SIZE_V, 4 },
+ { IMX290_X_OUT_SIZE, 1280 },
+ { IMX290_Y_OUT_SIZE, 720 },
{ IMX290_REG_8BIT(0x3012), 0x64 },
{ IMX290_REG_8BIT(0x3013), 0x00 },
- { IMX290_REG_8BIT(0x305c), 0x20 },
- { IMX290_REG_8BIT(0x305d), 0x00 },
- { IMX290_REG_8BIT(0x305e), 0x20 },
- { IMX290_REG_8BIT(0x305f), 0x01 },
- { IMX290_REG_8BIT(0x315e), 0x1a },
- { IMX290_REG_8BIT(0x3164), 0x1a },
+ { IMX290_INCKSEL1, 0x20 },
+ { IMX290_INCKSEL2, 0x00 },
+ { IMX290_INCKSEL3, 0x20 },
+ { IMX290_INCKSEL4, 0x01 },
+ { IMX290_INCKSEL5, 0x1a },
+ { IMX290_INCKSEL6, 0x1a },
{ IMX290_REG_8BIT(0x3480), 0x49 },
/* data rate settings */
- { IMX290_REG_8BIT(0x3405), 0x10 },
- { IMX290_REG_8BIT(0x3446), 0x4f },
- { IMX290_REG_8BIT(0x3447), 0x00 },
- { IMX290_REG_8BIT(0x3448), 0x2f },
- { IMX290_REG_8BIT(0x3449), 0x00 },
- { IMX290_REG_8BIT(0x344a), 0x17 },
- { IMX290_REG_8BIT(0x344b), 0x00 },
- { IMX290_REG_8BIT(0x344c), 0x17 },
- { IMX290_REG_8BIT(0x344d), 0x00 },
- { IMX290_REG_8BIT(0x344e), 0x17 },
- { IMX290_REG_8BIT(0x344f), 0x00 },
- { IMX290_REG_8BIT(0x3450), 0x57 },
- { IMX290_REG_8BIT(0x3451), 0x00 },
- { IMX290_REG_8BIT(0x3452), 0x17 },
- { IMX290_REG_8BIT(0x3453), 0x00 },
- { IMX290_REG_8BIT(0x3454), 0x17 },
- { IMX290_REG_8BIT(0x3455), 0x00 },
+ { IMX290_REPETITION, 0x10 },
+ { IMX290_TCLKPOST, 79 },
+ { IMX290_THSZERO, 47 },
+ { IMX290_THSPREPARE, 23 },
+ { IMX290_TCLKTRAIL, 23 },
+ { IMX290_THSTRAIL, 23 },
+ { IMX290_TCLKZERO, 87 },
+ { IMX290_TCLKPREPARE, 23 },
+ { IMX290_TLPX, 23 },
};
static const struct imx290_regval imx290_10bit_settings[] = {
- { IMX290_REG_8BIT(0x3005), 0x00},
- { IMX290_REG_8BIT(0x3046), 0x00},
- { IMX290_REG_8BIT(0x3129), 0x1d},
- { IMX290_REG_8BIT(0x317c), 0x12},
- { IMX290_REG_8BIT(0x31ec), 0x37},
- { IMX290_REG_8BIT(0x3441), 0x0a},
- { IMX290_REG_8BIT(0x3442), 0x0a},
- { IMX290_REG_8BIT(0x300a), 0x3c},
- { IMX290_REG_8BIT(0x300b), 0x00},
+ { IMX290_ADBIT, IMX290_ADBIT_10BIT },
+ { IMX290_OUT_CTRL, IMX290_ODBIT_10BIT },
+ { IMX290_ADBIT1, IMX290_ADBIT1_10BIT },
+ { IMX290_ADBIT2, IMX290_ADBIT2_10BIT },
+ { IMX290_ADBIT3, IMX290_ADBIT3_10BIT },
+ { IMX290_CSI_DT_FMT, IMX290_CSI_DT_FMT_RAW10 },
+ { IMX290_BLKLEVEL, 60 },
};
static const struct imx290_regval imx290_12bit_settings[] = {
- { IMX290_REG_8BIT(0x3005), 0x01 },
- { IMX290_REG_8BIT(0x3046), 0x01 },
- { IMX290_REG_8BIT(0x3129), 0x00 },
- { IMX290_REG_8BIT(0x317c), 0x00 },
- { IMX290_REG_8BIT(0x31ec), 0x0e },
- { IMX290_REG_8BIT(0x3441), 0x0c },
- { IMX290_REG_8BIT(0x3442), 0x0c },
- { IMX290_REG_8BIT(0x300a), 0xf0 },
- { IMX290_REG_8BIT(0x300b), 0x00 },
+ { IMX290_ADBIT, IMX290_ADBIT_12BIT },
+ { IMX290_OUT_CTRL, IMX290_ODBIT_12BIT },
+ { IMX290_ADBIT1, IMX290_ADBIT1_12BIT },
+ { IMX290_ADBIT2, IMX290_ADBIT2_12BIT },
+ { IMX290_ADBIT3, IMX290_ADBIT3_12BIT },
+ { IMX290_CSI_DT_FMT, IMX290_CSI_DT_FMT_RAW12 },
+ { IMX290_BLKLEVEL, 240 },
};
/* supported link frequencies */
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 11/20] media: i2c: imx290: Define more register macros
2022-10-16 6:15 ` [PATCH v2 11/20] media: i2c: imx290: Define more register macros Laurent Pinchart
@ 2022-10-17 6:07 ` Alexander Stein
2022-10-17 8:35 ` [PATCH v2.1 " Laurent Pinchart
1 sibling, 0 replies; 29+ messages in thread
From: Alexander Stein @ 2022-10-17 6:07 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Sakari Ailus, Manivannan Sadhasivam, Dave Stevenson
Hello Laurent,
thanks for the updated series.
Am Sonntag, 16. Oktober 2022, 08:15:14 CEST schrieb Laurent Pinchart:
> Define macros for all registers programmed by the driver for which
> documentation is available to increase readability. This starts making
> use of 16-bit registers in the register arrays, so the value field has
> to be increased to 32 bits.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/i2c/imx290.c | 219 +++++++++++++++++++++----------------
> 1 file changed, 124 insertions(+), 95 deletions(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index 0f26da5c2e54..93fd043669dc 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -31,14 +31,73 @@
> #define IMX290_STANDBY
IMX290_REG_8BIT(0x3000)
> #define IMX290_REGHOLD
IMX290_REG_8BIT(0x3001)
> #define IMX290_XMSTA
IMX290_REG_8BIT(0x3002)
> +#define IMX290_ADBIT
IMX290_REG_8BIT(0x3005)
> +#define IMX290_ADBIT_10BIT (0 << 0)
> +#define IMX290_ADBIT_12BIT (1 << 0)
> +#define IMX290_CTRL_07
IMX290_REG_8BIT(0x3007)
> +#define IMX290_VREVERSE
BIT(0)
> +#define IMX290_HREVERSE
BIT(1)
> +#define IMX290_WINMODE_1080P (0 << 4)
> +#define IMX290_WINMODE_720P (1 << 4)
> +#define IMX290_WINMODE_CROP (4 << 4)
> #define IMX290_FR_FDG_SEL
IMX290_REG_8BIT(0x3009)
> #define IMX290_BLKLEVEL
IMX290_REG_16BIT(0x300a)
> #define IMX290_GAIN
IMX290_REG_8BIT(0x3014)
> +#define IMX290_VMAX
IMX290_REG_24BIT(0x3018)
> #define IMX290_HMAX
IMX290_REG_16BIT(0x301c)
> +#define IMX290_SHS1
IMX290_REG_24BIT(0x3020)
> +#define IMX290_WINWV_OB
IMX290_REG_8BIT(0x303a)
> +#define IMX290_WINPV
IMX290_REG_16BIT(0x303c)
> +#define IMX290_WINWV
IMX290_REG_16BIT(0x303e)
> +#define IMX290_WINPH
IMX290_REG_16BIT(0x3040)
> +#define IMX290_WINWH
IMX290_REG_16BIT(0x3042)
> +#define IMX290_OUT_CTRL
IMX290_REG_8BIT(0x3046)
> +#define IMX290_ODBIT_10BIT (0 << 0)
> +#define IMX290_ODBIT_12BIT (1 << 0)
> +#define IMX290_OPORTSEL_PARALLEL (0x0 << 4)
> +#define IMX290_OPORTSEL_LVDS_2CH (0xd << 4)
> +#define IMX290_OPORTSEL_LVDS_4CH (0xe << 4)
> +#define IMX290_OPORTSEL_LVDS_8CH (0xf << 4)
> +#define IMX290_XSOUTSEL
IMX290_REG_8BIT(0x304b)
> +#define IMX290_XSOUTSEL_XVSOUTSEL_HIGH (0 << 0)
> +#define IMX290_XSOUTSEL_XVSOUTSEL_VSYNC (2 << 0)
> +#define IMX290_XSOUTSEL_XHSOUTSEL_HIGH (0 << 2)
> +#define IMX290_XSOUTSEL_XHSOUTSEL_HSYNC (2 << 2)
> +#define IMX290_INCKSEL1
IMX290_REG_8BIT(0x305c)
> +#define IMX290_INCKSEL2
IMX290_REG_8BIT(0x305d)
> +#define IMX290_INCKSEL3
IMX290_REG_8BIT(0x305e)
> +#define IMX290_INCKSEL4
IMX290_REG_8BIT(0x305f)
> #define IMX290_PGCTRL
IMX290_REG_8BIT(0x308c)
> +#define IMX290_ADBIT1
IMX290_REG_8BIT(0x3129)
> +#define IMX290_ADBIT1_10BIT 0x1d
> +#define IMX290_ADBIT1_12BIT 0x00
> +#define IMX290_INCKSEL5
IMX290_REG_8BIT(0x315e)
> +#define IMX290_INCKSEL6
IMX290_REG_8BIT(0x3164)
> +#define IMX290_ADBIT2
IMX290_REG_8BIT(0x317c)
> +#define IMX290_ADBIT2_10BIT 0x12
> +#define IMX290_ADBIT2_12BIT 0x00
> #define IMX290_CHIP_ID
IMX290_REG_16BIT(0x319a)
> +#define IMX290_ADBIT3
IMX290_REG_16BIT(0x31ec)
AFAICS this is only 8 bits wide.
Thanks
Alexander
> +#define IMX290_ADBIT3_10BIT 0x37
> +#define IMX290_ADBIT3_12BIT 0x0e
> +#define IMX290_REPETITION
IMX290_REG_8BIT(0x3405)
> #define IMX290_PHY_LANE_NUM
IMX290_REG_8BIT(0x3407)
> +#define IMX290_OPB_SIZE_V
IMX290_REG_8BIT(0x3414)
> +#define IMX290_Y_OUT_SIZE
IMX290_REG_16BIT(0x3418)
> +#define IMX290_CSI_DT_FMT
IMX290_REG_16BIT(0x3441)
> +#define IMX290_CSI_DT_FMT_RAW10
0x0a0a
> +#define IMX290_CSI_DT_FMT_RAW12
0x0c0c
> #define IMX290_CSI_LANE_MODE
IMX290_REG_8BIT(0x3443)
> +#define IMX290_EXTCK_FREQ
IMX290_REG_16BIT(0x3444)
> +#define IMX290_TCLKPOST
IMX290_REG_16BIT(0x3446)
> +#define IMX290_THSZERO
IMX290_REG_16BIT(0x3448)
> +#define IMX290_THSPREPARE
IMX290_REG_16BIT(0x344a)
> +#define IMX290_TCLKTRAIL
IMX290_REG_16BIT(0x344c)
> +#define IMX290_THSTRAIL
IMX290_REG_16BIT(0x344e)
> +#define IMX290_TCLKZERO
IMX290_REG_16BIT(0x3450)
> +#define IMX290_TCLKPREPARE
IMX290_REG_16BIT(0x3452)
> +#define IMX290_TLPX
IMX290_REG_16BIT(0x3454)
> +#define IMX290_X_OUT_SIZE
IMX290_REG_16BIT(0x3472)
>
> #define IMX290_PGCTRL_REGEN BIT(0)
> #define IMX290_PGCTRL_THRU BIT(1)
> @@ -54,7 +113,7 @@ static const char * const imx290_supply_name[] = {
>
> struct imx290_regval {
> u32 reg;
> - u8 val;
> + u32 val;
> };
>
> struct imx290_mode {
> @@ -116,22 +175,16 @@ static const char * const imx290_test_pattern_menu[] =
> { };
>
> static const struct imx290_regval imx290_global_init_settings[] = {
> - { IMX290_REG_8BIT(0x3007), 0x00 },
> - { IMX290_REG_8BIT(0x3018), 0x65 },
> - { IMX290_REG_8BIT(0x3019), 0x04 },
> - { IMX290_REG_8BIT(0x301a), 0x00 },
> - { IMX290_REG_8BIT(0x3444), 0x20 },
> - { IMX290_REG_8BIT(0x3445), 0x25 },
> - { IMX290_REG_8BIT(0x303a), 0x0c },
> - { IMX290_REG_8BIT(0x3040), 0x00 },
> - { IMX290_REG_8BIT(0x3041), 0x00 },
> - { IMX290_REG_8BIT(0x303c), 0x00 },
> - { IMX290_REG_8BIT(0x303d), 0x00 },
> - { IMX290_REG_8BIT(0x3042), 0x9c },
> - { IMX290_REG_8BIT(0x3043), 0x07 },
> - { IMX290_REG_8BIT(0x303e), 0x49 },
> - { IMX290_REG_8BIT(0x303f), 0x04 },
> - { IMX290_REG_8BIT(0x304b), 0x0a },
> + { IMX290_CTRL_07, IMX290_WINMODE_1080P },
> + { IMX290_VMAX, 1125 },
> + { IMX290_EXTCK_FREQ, 0x2520 },
> + { IMX290_WINWV_OB, 12 },
> + { IMX290_WINPH, 0 },
> + { IMX290_WINPV, 0 },
> + { IMX290_WINWH, 1948 },
> + { IMX290_WINWV, 1097 },
> + { IMX290_XSOUTSEL, IMX290_XSOUTSEL_XVSOUTSEL_VSYNC |
> + IMX290_XSOUTSEL_XHSOUTSEL_HSYNC },
> { IMX290_REG_8BIT(0x300f), 0x00 },
> { IMX290_REG_8BIT(0x3010), 0x21 },
> { IMX290_REG_8BIT(0x3012), 0x64 },
> @@ -177,102 +230,78 @@ static const struct imx290_regval
> imx290_global_init_settings[] = {
>
> static const struct imx290_regval imx290_1080p_settings[] = {
> /* mode settings */
> - { IMX290_REG_8BIT(0x3007), 0x00 },
> - { IMX290_REG_8BIT(0x303a), 0x0c },
> - { IMX290_REG_8BIT(0x3414), 0x0a },
> - { IMX290_REG_8BIT(0x3472), 0x80 },
> - { IMX290_REG_8BIT(0x3473), 0x07 },
> - { IMX290_REG_8BIT(0x3418), 0x38 },
> - { IMX290_REG_8BIT(0x3419), 0x04 },
> + { IMX290_CTRL_07, IMX290_WINMODE_1080P },
> + { IMX290_WINWV_OB, 12 },
> + { IMX290_OPB_SIZE_V, 10 },
> + { IMX290_X_OUT_SIZE, 1920 },
> + { IMX290_Y_OUT_SIZE, 1080 },
> { IMX290_REG_8BIT(0x3012), 0x64 },
> { IMX290_REG_8BIT(0x3013), 0x00 },
> - { IMX290_REG_8BIT(0x305c), 0x18 },
> - { IMX290_REG_8BIT(0x305d), 0x03 },
> - { IMX290_REG_8BIT(0x305e), 0x20 },
> - { IMX290_REG_8BIT(0x305f), 0x01 },
> - { IMX290_REG_8BIT(0x315e), 0x1a },
> - { IMX290_REG_8BIT(0x3164), 0x1a },
> + { IMX290_INCKSEL1, 0x18 },
> + { IMX290_INCKSEL2, 0x03 },
> + { IMX290_INCKSEL3, 0x20 },
> + { IMX290_INCKSEL4, 0x01 },
> + { IMX290_INCKSEL5, 0x1a },
> + { IMX290_INCKSEL6, 0x1a },
> { IMX290_REG_8BIT(0x3480), 0x49 },
> /* data rate settings */
> - { IMX290_REG_8BIT(0x3405), 0x10 },
> - { IMX290_REG_8BIT(0x3446), 0x57 },
> - { IMX290_REG_8BIT(0x3447), 0x00 },
> - { IMX290_REG_8BIT(0x3448), 0x37 },
> - { IMX290_REG_8BIT(0x3449), 0x00 },
> - { IMX290_REG_8BIT(0x344a), 0x1f },
> - { IMX290_REG_8BIT(0x344b), 0x00 },
> - { IMX290_REG_8BIT(0x344c), 0x1f },
> - { IMX290_REG_8BIT(0x344d), 0x00 },
> - { IMX290_REG_8BIT(0x344e), 0x1f },
> - { IMX290_REG_8BIT(0x344f), 0x00 },
> - { IMX290_REG_8BIT(0x3450), 0x77 },
> - { IMX290_REG_8BIT(0x3451), 0x00 },
> - { IMX290_REG_8BIT(0x3452), 0x1f },
> - { IMX290_REG_8BIT(0x3453), 0x00 },
> - { IMX290_REG_8BIT(0x3454), 0x17 },
> - { IMX290_REG_8BIT(0x3455), 0x00 },
> + { IMX290_REPETITION, 0x10 },
> + { IMX290_TCLKPOST, 87 },
> + { IMX290_THSZERO, 55 },
> + { IMX290_THSPREPARE, 31 },
> + { IMX290_TCLKTRAIL, 31 },
> + { IMX290_THSTRAIL, 31 },
> + { IMX290_TCLKZERO, 119 },
> + { IMX290_TCLKPREPARE, 31 },
> + { IMX290_TLPX, 23 },
> };
>
> static const struct imx290_regval imx290_720p_settings[] = {
> /* mode settings */
> - { IMX290_REG_8BIT(0x3007), 0x10 },
> - { IMX290_REG_8BIT(0x303a), 0x06 },
> - { IMX290_REG_8BIT(0x3414), 0x04 },
> - { IMX290_REG_8BIT(0x3472), 0x00 },
> - { IMX290_REG_8BIT(0x3473), 0x05 },
> - { IMX290_REG_8BIT(0x3418), 0xd0 },
> - { IMX290_REG_8BIT(0x3419), 0x02 },
> + { IMX290_CTRL_07, IMX290_WINMODE_720P },
> + { IMX290_WINWV_OB, 6 },
> + { IMX290_OPB_SIZE_V, 4 },
> + { IMX290_X_OUT_SIZE, 1280 },
> + { IMX290_Y_OUT_SIZE, 720 },
> { IMX290_REG_8BIT(0x3012), 0x64 },
> { IMX290_REG_8BIT(0x3013), 0x00 },
> - { IMX290_REG_8BIT(0x305c), 0x20 },
> - { IMX290_REG_8BIT(0x305d), 0x00 },
> - { IMX290_REG_8BIT(0x305e), 0x20 },
> - { IMX290_REG_8BIT(0x305f), 0x01 },
> - { IMX290_REG_8BIT(0x315e), 0x1a },
> - { IMX290_REG_8BIT(0x3164), 0x1a },
> + { IMX290_INCKSEL1, 0x20 },
> + { IMX290_INCKSEL2, 0x00 },
> + { IMX290_INCKSEL3, 0x20 },
> + { IMX290_INCKSEL4, 0x01 },
> + { IMX290_INCKSEL5, 0x1a },
> + { IMX290_INCKSEL6, 0x1a },
> { IMX290_REG_8BIT(0x3480), 0x49 },
> /* data rate settings */
> - { IMX290_REG_8BIT(0x3405), 0x10 },
> - { IMX290_REG_8BIT(0x3446), 0x4f },
> - { IMX290_REG_8BIT(0x3447), 0x00 },
> - { IMX290_REG_8BIT(0x3448), 0x2f },
> - { IMX290_REG_8BIT(0x3449), 0x00 },
> - { IMX290_REG_8BIT(0x344a), 0x17 },
> - { IMX290_REG_8BIT(0x344b), 0x00 },
> - { IMX290_REG_8BIT(0x344c), 0x17 },
> - { IMX290_REG_8BIT(0x344d), 0x00 },
> - { IMX290_REG_8BIT(0x344e), 0x17 },
> - { IMX290_REG_8BIT(0x344f), 0x00 },
> - { IMX290_REG_8BIT(0x3450), 0x57 },
> - { IMX290_REG_8BIT(0x3451), 0x00 },
> - { IMX290_REG_8BIT(0x3452), 0x17 },
> - { IMX290_REG_8BIT(0x3453), 0x00 },
> - { IMX290_REG_8BIT(0x3454), 0x17 },
> - { IMX290_REG_8BIT(0x3455), 0x00 },
> + { IMX290_REPETITION, 0x10 },
> + { IMX290_TCLKPOST, 79 },
> + { IMX290_THSZERO, 47 },
> + { IMX290_THSPREPARE, 23 },
> + { IMX290_TCLKTRAIL, 23 },
> + { IMX290_THSTRAIL, 23 },
> + { IMX290_TCLKZERO, 87 },
> + { IMX290_TCLKPREPARE, 23 },
> + { IMX290_TLPX, 23 },
> };
>
> static const struct imx290_regval imx290_10bit_settings[] = {
> - { IMX290_REG_8BIT(0x3005), 0x00},
> - { IMX290_REG_8BIT(0x3046), 0x00},
> - { IMX290_REG_8BIT(0x3129), 0x1d},
> - { IMX290_REG_8BIT(0x317c), 0x12},
> - { IMX290_REG_8BIT(0x31ec), 0x37},
> - { IMX290_REG_8BIT(0x3441), 0x0a},
> - { IMX290_REG_8BIT(0x3442), 0x0a},
> - { IMX290_REG_8BIT(0x300a), 0x3c},
> - { IMX290_REG_8BIT(0x300b), 0x00},
> + { IMX290_ADBIT, IMX290_ADBIT_10BIT },
> + { IMX290_OUT_CTRL, IMX290_ODBIT_10BIT },
> + { IMX290_ADBIT1, IMX290_ADBIT1_10BIT },
> + { IMX290_ADBIT2, IMX290_ADBIT2_10BIT },
> + { IMX290_ADBIT3, IMX290_ADBIT3_10BIT },
> + { IMX290_CSI_DT_FMT, IMX290_CSI_DT_FMT_RAW10 },
> + { IMX290_BLKLEVEL, 60 },
> };
>
> static const struct imx290_regval imx290_12bit_settings[] = {
> - { IMX290_REG_8BIT(0x3005), 0x01 },
> - { IMX290_REG_8BIT(0x3046), 0x01 },
> - { IMX290_REG_8BIT(0x3129), 0x00 },
> - { IMX290_REG_8BIT(0x317c), 0x00 },
> - { IMX290_REG_8BIT(0x31ec), 0x0e },
> - { IMX290_REG_8BIT(0x3441), 0x0c },
> - { IMX290_REG_8BIT(0x3442), 0x0c },
> - { IMX290_REG_8BIT(0x300a), 0xf0 },
> - { IMX290_REG_8BIT(0x300b), 0x00 },
> + { IMX290_ADBIT, IMX290_ADBIT_12BIT },
> + { IMX290_OUT_CTRL, IMX290_ODBIT_12BIT },
> + { IMX290_ADBIT1, IMX290_ADBIT1_12BIT },
> + { IMX290_ADBIT2, IMX290_ADBIT2_12BIT },
> + { IMX290_ADBIT3, IMX290_ADBIT3_12BIT },
> + { IMX290_CSI_DT_FMT, IMX290_CSI_DT_FMT_RAW12 },
> + { IMX290_BLKLEVEL, 240 },
> };
>
> /* supported link frequencies */
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH v2.1 11/20] media: i2c: imx290: Define more register macros
2022-10-16 6:15 ` [PATCH v2 11/20] media: i2c: imx290: Define more register macros Laurent Pinchart
2022-10-17 6:07 ` Alexander Stein
@ 2022-10-17 8:35 ` Laurent Pinchart
1 sibling, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-17 8:35 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Alexander Stein, Manivannan Sadhasivam,
Dave Stevenson
Define macros for all registers programmed by the driver for which
documentation is available to increase readability. This starts making
use of 16-bit registers in the register arrays, so the value field has
to be increased to 32 bits.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes since v2:
- Fix ADBIT3 register width
---
drivers/media/i2c/imx290.c | 219 +++++++++++++++++++++----------------
1 file changed, 124 insertions(+), 95 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index eb007c40ff55..c7b55953f5b1 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -31,14 +31,73 @@
#define IMX290_STANDBY IMX290_REG_8BIT(0x3000)
#define IMX290_REGHOLD IMX290_REG_8BIT(0x3001)
#define IMX290_XMSTA IMX290_REG_8BIT(0x3002)
+#define IMX290_ADBIT IMX290_REG_8BIT(0x3005)
+#define IMX290_ADBIT_10BIT (0 << 0)
+#define IMX290_ADBIT_12BIT (1 << 0)
+#define IMX290_CTRL_07 IMX290_REG_8BIT(0x3007)
+#define IMX290_VREVERSE BIT(0)
+#define IMX290_HREVERSE BIT(1)
+#define IMX290_WINMODE_1080P (0 << 4)
+#define IMX290_WINMODE_720P (1 << 4)
+#define IMX290_WINMODE_CROP (4 << 4)
#define IMX290_FR_FDG_SEL IMX290_REG_8BIT(0x3009)
#define IMX290_BLKLEVEL IMX290_REG_16BIT(0x300a)
#define IMX290_GAIN IMX290_REG_8BIT(0x3014)
+#define IMX290_VMAX IMX290_REG_24BIT(0x3018)
#define IMX290_HMAX IMX290_REG_16BIT(0x301c)
+#define IMX290_SHS1 IMX290_REG_24BIT(0x3020)
+#define IMX290_WINWV_OB IMX290_REG_8BIT(0x303a)
+#define IMX290_WINPV IMX290_REG_16BIT(0x303c)
+#define IMX290_WINWV IMX290_REG_16BIT(0x303e)
+#define IMX290_WINPH IMX290_REG_16BIT(0x3040)
+#define IMX290_WINWH IMX290_REG_16BIT(0x3042)
+#define IMX290_OUT_CTRL IMX290_REG_8BIT(0x3046)
+#define IMX290_ODBIT_10BIT (0 << 0)
+#define IMX290_ODBIT_12BIT (1 << 0)
+#define IMX290_OPORTSEL_PARALLEL (0x0 << 4)
+#define IMX290_OPORTSEL_LVDS_2CH (0xd << 4)
+#define IMX290_OPORTSEL_LVDS_4CH (0xe << 4)
+#define IMX290_OPORTSEL_LVDS_8CH (0xf << 4)
+#define IMX290_XSOUTSEL IMX290_REG_8BIT(0x304b)
+#define IMX290_XSOUTSEL_XVSOUTSEL_HIGH (0 << 0)
+#define IMX290_XSOUTSEL_XVSOUTSEL_VSYNC (2 << 0)
+#define IMX290_XSOUTSEL_XHSOUTSEL_HIGH (0 << 2)
+#define IMX290_XSOUTSEL_XHSOUTSEL_HSYNC (2 << 2)
+#define IMX290_INCKSEL1 IMX290_REG_8BIT(0x305c)
+#define IMX290_INCKSEL2 IMX290_REG_8BIT(0x305d)
+#define IMX290_INCKSEL3 IMX290_REG_8BIT(0x305e)
+#define IMX290_INCKSEL4 IMX290_REG_8BIT(0x305f)
#define IMX290_PGCTRL IMX290_REG_8BIT(0x308c)
+#define IMX290_ADBIT1 IMX290_REG_8BIT(0x3129)
+#define IMX290_ADBIT1_10BIT 0x1d
+#define IMX290_ADBIT1_12BIT 0x00
+#define IMX290_INCKSEL5 IMX290_REG_8BIT(0x315e)
+#define IMX290_INCKSEL6 IMX290_REG_8BIT(0x3164)
+#define IMX290_ADBIT2 IMX290_REG_8BIT(0x317c)
+#define IMX290_ADBIT2_10BIT 0x12
+#define IMX290_ADBIT2_12BIT 0x00
#define IMX290_CHIP_ID IMX290_REG_16BIT(0x319a)
+#define IMX290_ADBIT3 IMX290_REG_8BIT(0x31ec)
+#define IMX290_ADBIT3_10BIT 0x37
+#define IMX290_ADBIT3_12BIT 0x0e
+#define IMX290_REPETITION IMX290_REG_8BIT(0x3405)
#define IMX290_PHY_LANE_NUM IMX290_REG_8BIT(0x3407)
+#define IMX290_OPB_SIZE_V IMX290_REG_8BIT(0x3414)
+#define IMX290_Y_OUT_SIZE IMX290_REG_16BIT(0x3418)
+#define IMX290_CSI_DT_FMT IMX290_REG_16BIT(0x3441)
+#define IMX290_CSI_DT_FMT_RAW10 0x0a0a
+#define IMX290_CSI_DT_FMT_RAW12 0x0c0c
#define IMX290_CSI_LANE_MODE IMX290_REG_8BIT(0x3443)
+#define IMX290_EXTCK_FREQ IMX290_REG_16BIT(0x3444)
+#define IMX290_TCLKPOST IMX290_REG_16BIT(0x3446)
+#define IMX290_THSZERO IMX290_REG_16BIT(0x3448)
+#define IMX290_THSPREPARE IMX290_REG_16BIT(0x344a)
+#define IMX290_TCLKTRAIL IMX290_REG_16BIT(0x344c)
+#define IMX290_THSTRAIL IMX290_REG_16BIT(0x344e)
+#define IMX290_TCLKZERO IMX290_REG_16BIT(0x3450)
+#define IMX290_TCLKPREPARE IMX290_REG_16BIT(0x3452)
+#define IMX290_TLPX IMX290_REG_16BIT(0x3454)
+#define IMX290_X_OUT_SIZE IMX290_REG_16BIT(0x3472)
#define IMX290_PGCTRL_REGEN BIT(0)
#define IMX290_PGCTRL_THRU BIT(1)
@@ -54,7 +113,7 @@ static const char * const imx290_supply_name[] = {
struct imx290_regval {
u32 reg;
- u8 val;
+ u32 val;
};
struct imx290_mode {
@@ -116,22 +175,16 @@ static const char * const imx290_test_pattern_menu[] = {
};
static const struct imx290_regval imx290_global_init_settings[] = {
- { IMX290_REG_8BIT(0x3007), 0x00 },
- { IMX290_REG_8BIT(0x3018), 0x65 },
- { IMX290_REG_8BIT(0x3019), 0x04 },
- { IMX290_REG_8BIT(0x301a), 0x00 },
- { IMX290_REG_8BIT(0x3444), 0x20 },
- { IMX290_REG_8BIT(0x3445), 0x25 },
- { IMX290_REG_8BIT(0x303a), 0x0c },
- { IMX290_REG_8BIT(0x3040), 0x00 },
- { IMX290_REG_8BIT(0x3041), 0x00 },
- { IMX290_REG_8BIT(0x303c), 0x00 },
- { IMX290_REG_8BIT(0x303d), 0x00 },
- { IMX290_REG_8BIT(0x3042), 0x9c },
- { IMX290_REG_8BIT(0x3043), 0x07 },
- { IMX290_REG_8BIT(0x303e), 0x49 },
- { IMX290_REG_8BIT(0x303f), 0x04 },
- { IMX290_REG_8BIT(0x304b), 0x0a },
+ { IMX290_CTRL_07, IMX290_WINMODE_1080P },
+ { IMX290_VMAX, 1125 },
+ { IMX290_EXTCK_FREQ, 0x2520 },
+ { IMX290_WINWV_OB, 12 },
+ { IMX290_WINPH, 0 },
+ { IMX290_WINPV, 0 },
+ { IMX290_WINWH, 1948 },
+ { IMX290_WINWV, 1097 },
+ { IMX290_XSOUTSEL, IMX290_XSOUTSEL_XVSOUTSEL_VSYNC |
+ IMX290_XSOUTSEL_XHSOUTSEL_HSYNC },
{ IMX290_REG_8BIT(0x300f), 0x00 },
{ IMX290_REG_8BIT(0x3010), 0x21 },
{ IMX290_REG_8BIT(0x3012), 0x64 },
@@ -177,102 +230,78 @@ static const struct imx290_regval imx290_global_init_settings[] = {
static const struct imx290_regval imx290_1080p_settings[] = {
/* mode settings */
- { IMX290_REG_8BIT(0x3007), 0x00 },
- { IMX290_REG_8BIT(0x303a), 0x0c },
- { IMX290_REG_8BIT(0x3414), 0x0a },
- { IMX290_REG_8BIT(0x3472), 0x80 },
- { IMX290_REG_8BIT(0x3473), 0x07 },
- { IMX290_REG_8BIT(0x3418), 0x38 },
- { IMX290_REG_8BIT(0x3419), 0x04 },
+ { IMX290_CTRL_07, IMX290_WINMODE_1080P },
+ { IMX290_WINWV_OB, 12 },
+ { IMX290_OPB_SIZE_V, 10 },
+ { IMX290_X_OUT_SIZE, 1920 },
+ { IMX290_Y_OUT_SIZE, 1080 },
{ IMX290_REG_8BIT(0x3012), 0x64 },
{ IMX290_REG_8BIT(0x3013), 0x00 },
- { IMX290_REG_8BIT(0x305c), 0x18 },
- { IMX290_REG_8BIT(0x305d), 0x03 },
- { IMX290_REG_8BIT(0x305e), 0x20 },
- { IMX290_REG_8BIT(0x305f), 0x01 },
- { IMX290_REG_8BIT(0x315e), 0x1a },
- { IMX290_REG_8BIT(0x3164), 0x1a },
+ { IMX290_INCKSEL1, 0x18 },
+ { IMX290_INCKSEL2, 0x03 },
+ { IMX290_INCKSEL3, 0x20 },
+ { IMX290_INCKSEL4, 0x01 },
+ { IMX290_INCKSEL5, 0x1a },
+ { IMX290_INCKSEL6, 0x1a },
{ IMX290_REG_8BIT(0x3480), 0x49 },
/* data rate settings */
- { IMX290_REG_8BIT(0x3405), 0x10 },
- { IMX290_REG_8BIT(0x3446), 0x57 },
- { IMX290_REG_8BIT(0x3447), 0x00 },
- { IMX290_REG_8BIT(0x3448), 0x37 },
- { IMX290_REG_8BIT(0x3449), 0x00 },
- { IMX290_REG_8BIT(0x344a), 0x1f },
- { IMX290_REG_8BIT(0x344b), 0x00 },
- { IMX290_REG_8BIT(0x344c), 0x1f },
- { IMX290_REG_8BIT(0x344d), 0x00 },
- { IMX290_REG_8BIT(0x344e), 0x1f },
- { IMX290_REG_8BIT(0x344f), 0x00 },
- { IMX290_REG_8BIT(0x3450), 0x77 },
- { IMX290_REG_8BIT(0x3451), 0x00 },
- { IMX290_REG_8BIT(0x3452), 0x1f },
- { IMX290_REG_8BIT(0x3453), 0x00 },
- { IMX290_REG_8BIT(0x3454), 0x17 },
- { IMX290_REG_8BIT(0x3455), 0x00 },
+ { IMX290_REPETITION, 0x10 },
+ { IMX290_TCLKPOST, 87 },
+ { IMX290_THSZERO, 55 },
+ { IMX290_THSPREPARE, 31 },
+ { IMX290_TCLKTRAIL, 31 },
+ { IMX290_THSTRAIL, 31 },
+ { IMX290_TCLKZERO, 119 },
+ { IMX290_TCLKPREPARE, 31 },
+ { IMX290_TLPX, 23 },
};
static const struct imx290_regval imx290_720p_settings[] = {
/* mode settings */
- { IMX290_REG_8BIT(0x3007), 0x10 },
- { IMX290_REG_8BIT(0x303a), 0x06 },
- { IMX290_REG_8BIT(0x3414), 0x04 },
- { IMX290_REG_8BIT(0x3472), 0x00 },
- { IMX290_REG_8BIT(0x3473), 0x05 },
- { IMX290_REG_8BIT(0x3418), 0xd0 },
- { IMX290_REG_8BIT(0x3419), 0x02 },
+ { IMX290_CTRL_07, IMX290_WINMODE_720P },
+ { IMX290_WINWV_OB, 6 },
+ { IMX290_OPB_SIZE_V, 4 },
+ { IMX290_X_OUT_SIZE, 1280 },
+ { IMX290_Y_OUT_SIZE, 720 },
{ IMX290_REG_8BIT(0x3012), 0x64 },
{ IMX290_REG_8BIT(0x3013), 0x00 },
- { IMX290_REG_8BIT(0x305c), 0x20 },
- { IMX290_REG_8BIT(0x305d), 0x00 },
- { IMX290_REG_8BIT(0x305e), 0x20 },
- { IMX290_REG_8BIT(0x305f), 0x01 },
- { IMX290_REG_8BIT(0x315e), 0x1a },
- { IMX290_REG_8BIT(0x3164), 0x1a },
+ { IMX290_INCKSEL1, 0x20 },
+ { IMX290_INCKSEL2, 0x00 },
+ { IMX290_INCKSEL3, 0x20 },
+ { IMX290_INCKSEL4, 0x01 },
+ { IMX290_INCKSEL5, 0x1a },
+ { IMX290_INCKSEL6, 0x1a },
{ IMX290_REG_8BIT(0x3480), 0x49 },
/* data rate settings */
- { IMX290_REG_8BIT(0x3405), 0x10 },
- { IMX290_REG_8BIT(0x3446), 0x4f },
- { IMX290_REG_8BIT(0x3447), 0x00 },
- { IMX290_REG_8BIT(0x3448), 0x2f },
- { IMX290_REG_8BIT(0x3449), 0x00 },
- { IMX290_REG_8BIT(0x344a), 0x17 },
- { IMX290_REG_8BIT(0x344b), 0x00 },
- { IMX290_REG_8BIT(0x344c), 0x17 },
- { IMX290_REG_8BIT(0x344d), 0x00 },
- { IMX290_REG_8BIT(0x344e), 0x17 },
- { IMX290_REG_8BIT(0x344f), 0x00 },
- { IMX290_REG_8BIT(0x3450), 0x57 },
- { IMX290_REG_8BIT(0x3451), 0x00 },
- { IMX290_REG_8BIT(0x3452), 0x17 },
- { IMX290_REG_8BIT(0x3453), 0x00 },
- { IMX290_REG_8BIT(0x3454), 0x17 },
- { IMX290_REG_8BIT(0x3455), 0x00 },
+ { IMX290_REPETITION, 0x10 },
+ { IMX290_TCLKPOST, 79 },
+ { IMX290_THSZERO, 47 },
+ { IMX290_THSPREPARE, 23 },
+ { IMX290_TCLKTRAIL, 23 },
+ { IMX290_THSTRAIL, 23 },
+ { IMX290_TCLKZERO, 87 },
+ { IMX290_TCLKPREPARE, 23 },
+ { IMX290_TLPX, 23 },
};
static const struct imx290_regval imx290_10bit_settings[] = {
- { IMX290_REG_8BIT(0x3005), 0x00},
- { IMX290_REG_8BIT(0x3046), 0x00},
- { IMX290_REG_8BIT(0x3129), 0x1d},
- { IMX290_REG_8BIT(0x317c), 0x12},
- { IMX290_REG_8BIT(0x31ec), 0x37},
- { IMX290_REG_8BIT(0x3441), 0x0a},
- { IMX290_REG_8BIT(0x3442), 0x0a},
- { IMX290_REG_8BIT(0x300a), 0x3c},
- { IMX290_REG_8BIT(0x300b), 0x00},
+ { IMX290_ADBIT, IMX290_ADBIT_10BIT },
+ { IMX290_OUT_CTRL, IMX290_ODBIT_10BIT },
+ { IMX290_ADBIT1, IMX290_ADBIT1_10BIT },
+ { IMX290_ADBIT2, IMX290_ADBIT2_10BIT },
+ { IMX290_ADBIT3, IMX290_ADBIT3_10BIT },
+ { IMX290_CSI_DT_FMT, IMX290_CSI_DT_FMT_RAW10 },
+ { IMX290_BLKLEVEL, 60 },
};
static const struct imx290_regval imx290_12bit_settings[] = {
- { IMX290_REG_8BIT(0x3005), 0x01 },
- { IMX290_REG_8BIT(0x3046), 0x01 },
- { IMX290_REG_8BIT(0x3129), 0x00 },
- { IMX290_REG_8BIT(0x317c), 0x00 },
- { IMX290_REG_8BIT(0x31ec), 0x0e },
- { IMX290_REG_8BIT(0x3441), 0x0c },
- { IMX290_REG_8BIT(0x3442), 0x0c },
- { IMX290_REG_8BIT(0x300a), 0xf0 },
- { IMX290_REG_8BIT(0x300b), 0x00 },
+ { IMX290_ADBIT, IMX290_ADBIT_12BIT },
+ { IMX290_OUT_CTRL, IMX290_ODBIT_12BIT },
+ { IMX290_ADBIT1, IMX290_ADBIT1_12BIT },
+ { IMX290_ADBIT2, IMX290_ADBIT2_12BIT },
+ { IMX290_ADBIT3, IMX290_ADBIT3_12BIT },
+ { IMX290_CSI_DT_FMT, IMX290_CSI_DT_FMT_RAW12 },
+ { IMX290_BLKLEVEL, 240 },
};
/* supported link frequencies */
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 12/20] media: i2c: imx290: Add exposure time control
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (10 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 11/20] media: i2c: imx290: Define more register macros Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 13/20] media: i2c: imx290: Fix max gain value Laurent Pinchart
` (7 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Support configuring the exposure time, which is expressed as the
complement of the exposure time (frame period minus integration time).
The frame period is currently fixed.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
drivers/media/i2c/imx290.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 93fd043669dc..a3d1394819ec 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -103,6 +103,8 @@
#define IMX290_PGCTRL_THRU BIT(1)
#define IMX290_PGCTRL_MODE(n) ((n) << 4)
+#define IMX290_VMAX_DEFAULT 1125
+
static const char * const imx290_supply_name[] = {
"vdda",
"vddd",
@@ -176,7 +178,7 @@ static const char * const imx290_test_pattern_menu[] = {
static const struct imx290_regval imx290_global_init_settings[] = {
{ IMX290_CTRL_07, IMX290_WINMODE_1080P },
- { IMX290_VMAX, 1125 },
+ { IMX290_VMAX, IMX290_VMAX_DEFAULT },
{ IMX290_EXTCK_FREQ, 0x2520 },
{ IMX290_WINWV_OB, 12 },
{ IMX290_WINPH, 0 },
@@ -480,6 +482,12 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_GAIN:
ret = imx290_write(imx290, IMX290_GAIN, ctrl->val, NULL);
break;
+
+ case V4L2_CID_EXPOSURE:
+ ret = imx290_write(imx290, IMX290_SHS1,
+ IMX290_VMAX_DEFAULT - ctrl->val - 1, NULL);
+ break;
+
case V4L2_CID_TEST_PATTERN:
if (ctrl->val) {
imx290_write(imx290, IMX290_BLKLEVEL, 0, &ret);
@@ -1008,12 +1016,16 @@ static int imx290_probe(struct i2c_client *client)
*/
imx290_entity_init_cfg(&imx290->sd, NULL);
- v4l2_ctrl_handler_init(&imx290->ctrls, 4);
+ v4l2_ctrl_handler_init(&imx290->ctrls, 5);
imx290->ctrls.lock = &imx290->lock;
v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
V4L2_CID_GAIN, 0, 72, 1, 0);
+ v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
+ V4L2_CID_EXPOSURE, 1, IMX290_VMAX_DEFAULT - 2, 1,
+ IMX290_VMAX_DEFAULT - 2);
+
imx290->link_freq =
v4l2_ctrl_new_int_menu(&imx290->ctrls, &imx290_ctrl_ops,
V4L2_CID_LINK_FREQ,
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 13/20] media: i2c: imx290: Fix max gain value
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (11 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 12/20] media: i2c: imx290: Add exposure time control Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 14/20] media: i2c: imx290: Split control initialization to separate function Laurent Pinchart
` (6 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
The gain is expressed in multiple of 0.3dB, as a value between 0.0dB
and 72.0dB. Gains between 0.0dB and 30.0dB (included) apply analog gain
only, higher gains from 30.3dB to 72dB apply additional digital gain.
The maximum gain value is erroneously set to 72. Increase it to 100 to
cover the whole analog gain range. Support for digital gain can be added
separately if needed.
The IMX327 and IMX462 are largely compatible with the IMX290, but have
an analog gain range of 0.0dB to 29.4dB and 42dB of digital gain. When
support for those sensors gets added to the driver, the gain control
should be adjusted accordingly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes since v1:
- Limit gain to 100
- Add comment
---
drivers/media/i2c/imx290.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index a3d1394819ec..43ac6244c3a2 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -1019,8 +1019,21 @@ static int imx290_probe(struct i2c_client *client)
v4l2_ctrl_handler_init(&imx290->ctrls, 5);
imx290->ctrls.lock = &imx290->lock;
+ /*
+ * The sensor has an analog gain and a digital gain, both controlled
+ * through a single gain value, expressed in 0.3dB increments. Values
+ * from 0.0dB (0) to 30.0dB (100) apply analog gain only, higher values
+ * up to 72.0dB (240) add further digital gain. Limit the range to
+ * analog gain only, support for digital gain can be added separately
+ * if needed.
+ *
+ * The IMX327 and IMX462 are largely compatible with the IMX290, but
+ * have an analog gain range of 0.0dB to 29.4dB and 42dB of digital
+ * gain. When support for those sensors gets added to the driver, the
+ * gain control should be adjusted accordingly.
+ */
v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
- V4L2_CID_GAIN, 0, 72, 1, 0);
+ V4L2_CID_GAIN, 0, 100, 1, 0);
v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
V4L2_CID_EXPOSURE, 1, IMX290_VMAX_DEFAULT - 2, 1,
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 14/20] media: i2c: imx290: Split control initialization to separate function
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (12 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 13/20] media: i2c: imx290: Fix max gain value Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 15/20] media: i2c: imx290: Implement HBLANK and VBLANK controls Laurent Pinchart
` (5 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
The imx290_probe() function is too large. Split control initialzation to
a dedicated function to increase code readability.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/media/i2c/imx290.c | 109 +++++++++++++++++++++----------------
1 file changed, 61 insertions(+), 48 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 43ac6244c3a2..5deeab594e9b 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -878,6 +878,62 @@ static const struct media_entity_operations imx290_subdev_entity_ops = {
.link_validate = v4l2_subdev_link_validate,
};
+static int imx290_ctrl_init(struct imx290 *imx290)
+{
+ int ret;
+
+ v4l2_ctrl_handler_init(&imx290->ctrls, 5);
+ imx290->ctrls.lock = &imx290->lock;
+
+ /*
+ * The sensor has an analog gain and a digital gain, both controlled
+ * through a single gain value, expressed in 0.3dB increments. Values
+ * from 0.0dB (0) to 30.0dB (100) apply analog gain only, higher values
+ * up to 72.0dB (240) add further digital gain. Limit the range to
+ * analog gain only, support for digital gain can be added separately
+ * if needed.
+ *
+ * The IMX327 and IMX462 are largely compatible with the IMX290, but
+ * have an analog gain range of 0.0dB to 29.4dB and 42dB of digital
+ * gain. When support for those sensors gets added to the driver, the
+ * gain control should be adjusted accordingly.
+ */
+ v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
+ V4L2_CID_GAIN, 0, 100, 1, 0);
+
+ v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
+ V4L2_CID_EXPOSURE, 1, IMX290_VMAX_DEFAULT - 2, 1,
+ IMX290_VMAX_DEFAULT - 2);
+
+ imx290->link_freq =
+ v4l2_ctrl_new_int_menu(&imx290->ctrls, &imx290_ctrl_ops,
+ V4L2_CID_LINK_FREQ,
+ imx290_link_freqs_num(imx290) - 1, 0,
+ imx290_link_freqs_ptr(imx290));
+ if (imx290->link_freq)
+ imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+ imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
+ V4L2_CID_PIXEL_RATE,
+ 1, INT_MAX, 1,
+ imx290_calc_pixel_rate(imx290));
+
+ v4l2_ctrl_new_std_menu_items(&imx290->ctrls, &imx290_ctrl_ops,
+ V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(imx290_test_pattern_menu) - 1,
+ 0, 0, imx290_test_pattern_menu);
+
+ imx290->sd.ctrl_handler = &imx290->ctrls;
+
+ if (imx290->ctrls.error) {
+ ret = imx290->ctrls.error;
+ v4l2_ctrl_handler_free(&imx290->ctrls);
+ return ret;
+ }
+
+ return 0;
+}
+
/*
* Returns 0 if all link frequencies used by the driver for the given number
* of MIPI data lanes are mentioned in the device tree, or the value of the
@@ -1016,54 +1072,10 @@ static int imx290_probe(struct i2c_client *client)
*/
imx290_entity_init_cfg(&imx290->sd, NULL);
- v4l2_ctrl_handler_init(&imx290->ctrls, 5);
- imx290->ctrls.lock = &imx290->lock;
-
- /*
- * The sensor has an analog gain and a digital gain, both controlled
- * through a single gain value, expressed in 0.3dB increments. Values
- * from 0.0dB (0) to 30.0dB (100) apply analog gain only, higher values
- * up to 72.0dB (240) add further digital gain. Limit the range to
- * analog gain only, support for digital gain can be added separately
- * if needed.
- *
- * The IMX327 and IMX462 are largely compatible with the IMX290, but
- * have an analog gain range of 0.0dB to 29.4dB and 42dB of digital
- * gain. When support for those sensors gets added to the driver, the
- * gain control should be adjusted accordingly.
- */
- v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
- V4L2_CID_GAIN, 0, 100, 1, 0);
-
- v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
- V4L2_CID_EXPOSURE, 1, IMX290_VMAX_DEFAULT - 2, 1,
- IMX290_VMAX_DEFAULT - 2);
-
- imx290->link_freq =
- v4l2_ctrl_new_int_menu(&imx290->ctrls, &imx290_ctrl_ops,
- V4L2_CID_LINK_FREQ,
- imx290_link_freqs_num(imx290) - 1, 0,
- imx290_link_freqs_ptr(imx290));
- if (imx290->link_freq)
- imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
-
- imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
- V4L2_CID_PIXEL_RATE,
- 1, INT_MAX, 1,
- imx290_calc_pixel_rate(imx290));
-
- v4l2_ctrl_new_std_menu_items(&imx290->ctrls, &imx290_ctrl_ops,
- V4L2_CID_TEST_PATTERN,
- ARRAY_SIZE(imx290_test_pattern_menu) - 1,
- 0, 0, imx290_test_pattern_menu);
-
- imx290->sd.ctrl_handler = &imx290->ctrls;
-
- if (imx290->ctrls.error) {
- dev_err(dev, "Control initialization error %d\n",
- imx290->ctrls.error);
- ret = imx290->ctrls.error;
- goto free_ctrl;
+ ret = imx290_ctrl_init(imx290);
+ if (ret < 0) {
+ dev_err(dev, "Control initialization error %d\n", ret);
+ goto free_mutex;
}
v4l2_i2c_subdev_init(&imx290->sd, client, &imx290_subdev_ops);
@@ -1104,6 +1116,7 @@ static int imx290_probe(struct i2c_client *client)
media_entity_cleanup(&imx290->sd.entity);
free_ctrl:
v4l2_ctrl_handler_free(&imx290->ctrls);
+free_mutex:
mutex_destroy(&imx290->lock);
free_err:
v4l2_fwnode_endpoint_free(&ep);
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 15/20] media: i2c: imx290: Implement HBLANK and VBLANK controls
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (13 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 14/20] media: i2c: imx290: Split control initialization to separate function Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 16/20] media: i2c: imx290: Create controls for fwnode properties Laurent Pinchart
` (4 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Add support for the V4L2_CID_HBLANK and V4L2_CID_VBLANK controls to the
imx290 driver. Make the controls read-only to start with, to report the
values to userspace for timing calculation.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:
- Drop incorrect comment
---
drivers/media/i2c/imx290.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 5deeab594e9b..c5cecfd5d24c 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -146,6 +146,8 @@ struct imx290 {
struct v4l2_ctrl_handler ctrls;
struct v4l2_ctrl *link_freq;
struct v4l2_ctrl *pixel_rate;
+ struct v4l2_ctrl *hblank;
+ struct v4l2_ctrl *vblank;
struct mutex lock;
};
@@ -642,6 +644,20 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
if (imx290->pixel_rate)
__v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate,
imx290_calc_pixel_rate(imx290));
+
+ if (imx290->hblank) {
+ unsigned int hblank = mode->hmax - mode->width;
+
+ __v4l2_ctrl_modify_range(imx290->hblank, hblank, hblank,
+ 1, hblank);
+ }
+
+ if (imx290->vblank) {
+ unsigned int vblank = IMX290_VMAX_DEFAULT - mode->height;
+
+ __v4l2_ctrl_modify_range(imx290->vblank, vblank, vblank,
+ 1, vblank);
+ }
}
*format = fmt->format;
@@ -880,9 +896,10 @@ static const struct media_entity_operations imx290_subdev_entity_ops = {
static int imx290_ctrl_init(struct imx290 *imx290)
{
+ unsigned int blank;
int ret;
- v4l2_ctrl_handler_init(&imx290->ctrls, 5);
+ v4l2_ctrl_handler_init(&imx290->ctrls, 7);
imx290->ctrls.lock = &imx290->lock;
/*
@@ -923,6 +940,20 @@ static int imx290_ctrl_init(struct imx290 *imx290)
ARRAY_SIZE(imx290_test_pattern_menu) - 1,
0, 0, imx290_test_pattern_menu);
+ blank = imx290->current_mode->hmax - imx290->current_mode->width;
+ imx290->hblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
+ V4L2_CID_HBLANK, blank, blank, 1,
+ blank);
+ if (imx290->hblank)
+ imx290->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+ blank = IMX290_VMAX_DEFAULT - imx290->current_mode->height;
+ imx290->vblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
+ V4L2_CID_VBLANK, blank, blank, 1,
+ blank);
+ if (imx290->vblank)
+ imx290->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
imx290->sd.ctrl_handler = &imx290->ctrls;
if (imx290->ctrls.error) {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 16/20] media: i2c: imx290: Create controls for fwnode properties
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (14 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 15/20] media: i2c: imx290: Implement HBLANK and VBLANK controls Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 17/20] media: i2c: imx290: Move registers with fixed value to init array Laurent Pinchart
` (3 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Create the V4L2_CID_ORIENTATION and V4L2_CID_ROTATION controls to
expose the corresponding fwnode properties.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/media/i2c/imx290.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index c5cecfd5d24c..93eab6c96ca0 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -896,10 +896,15 @@ static const struct media_entity_operations imx290_subdev_entity_ops = {
static int imx290_ctrl_init(struct imx290 *imx290)
{
+ struct v4l2_fwnode_device_properties props;
unsigned int blank;
int ret;
- v4l2_ctrl_handler_init(&imx290->ctrls, 7);
+ ret = v4l2_fwnode_device_parse(imx290->dev, &props);
+ if (ret < 0)
+ return ret;
+
+ v4l2_ctrl_handler_init(&imx290->ctrls, 9);
imx290->ctrls.lock = &imx290->lock;
/*
@@ -954,6 +959,9 @@ static int imx290_ctrl_init(struct imx290 *imx290)
if (imx290->vblank)
imx290->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+ v4l2_ctrl_new_fwnode_properties(&imx290->ctrls, &imx290_ctrl_ops,
+ &props);
+
imx290->sd.ctrl_handler = &imx290->ctrls;
if (imx290->ctrls.error) {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 17/20] media: i2c: imx290: Move registers with fixed value to init array
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (15 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 16/20] media: i2c: imx290: Create controls for fwnode properties Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-17 5:52 ` Alexander Stein
2022-10-16 6:15 ` [PATCH v2 18/20] media: i2c: imx290: Factor out format retrieval to separate function Laurent Pinchart
` (2 subsequent siblings)
19 siblings, 1 reply; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Registers 0x3012, 0x3013 and 0x3480 are not documented and are set in
the per-mode register arrays with values indentical for all modes. Move
them to the common array.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/i2c/imx290.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 93eab6c96ca0..0b34d60f8ce2 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -192,6 +192,7 @@ static const struct imx290_regval imx290_global_init_settings[] = {
{ IMX290_REG_8BIT(0x300f), 0x00 },
{ IMX290_REG_8BIT(0x3010), 0x21 },
{ IMX290_REG_8BIT(0x3012), 0x64 },
+ { IMX290_REG_8BIT(0x3013), 0x00 },
{ IMX290_REG_8BIT(0x3016), 0x09 },
{ IMX290_REG_8BIT(0x3070), 0x02 },
{ IMX290_REG_8BIT(0x3071), 0x11 },
@@ -230,6 +231,7 @@ static const struct imx290_regval imx290_global_init_settings[] = {
{ IMX290_REG_8BIT(0x33b0), 0x50 },
{ IMX290_REG_8BIT(0x33b2), 0x1a },
{ IMX290_REG_8BIT(0x33b3), 0x04 },
+ { IMX290_REG_8BIT(0x3480), 0x49 },
};
static const struct imx290_regval imx290_1080p_settings[] = {
@@ -239,15 +241,12 @@ static const struct imx290_regval imx290_1080p_settings[] = {
{ IMX290_OPB_SIZE_V, 10 },
{ IMX290_X_OUT_SIZE, 1920 },
{ IMX290_Y_OUT_SIZE, 1080 },
- { IMX290_REG_8BIT(0x3012), 0x64 },
- { IMX290_REG_8BIT(0x3013), 0x00 },
{ IMX290_INCKSEL1, 0x18 },
{ IMX290_INCKSEL2, 0x03 },
{ IMX290_INCKSEL3, 0x20 },
{ IMX290_INCKSEL4, 0x01 },
{ IMX290_INCKSEL5, 0x1a },
{ IMX290_INCKSEL6, 0x1a },
- { IMX290_REG_8BIT(0x3480), 0x49 },
/* data rate settings */
{ IMX290_REPETITION, 0x10 },
{ IMX290_TCLKPOST, 87 },
@@ -267,15 +266,12 @@ static const struct imx290_regval imx290_720p_settings[] = {
{ IMX290_OPB_SIZE_V, 4 },
{ IMX290_X_OUT_SIZE, 1280 },
{ IMX290_Y_OUT_SIZE, 720 },
- { IMX290_REG_8BIT(0x3012), 0x64 },
- { IMX290_REG_8BIT(0x3013), 0x00 },
{ IMX290_INCKSEL1, 0x20 },
{ IMX290_INCKSEL2, 0x00 },
{ IMX290_INCKSEL3, 0x20 },
{ IMX290_INCKSEL4, 0x01 },
{ IMX290_INCKSEL5, 0x1a },
{ IMX290_INCKSEL6, 0x1a },
- { IMX290_REG_8BIT(0x3480), 0x49 },
/* data rate settings */
{ IMX290_REPETITION, 0x10 },
{ IMX290_TCLKPOST, 79 },
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 17/20] media: i2c: imx290: Move registers with fixed value to init array
2022-10-16 6:15 ` [PATCH v2 17/20] media: i2c: imx290: Move registers with fixed value to init array Laurent Pinchart
@ 2022-10-17 5:52 ` Alexander Stein
0 siblings, 0 replies; 29+ messages in thread
From: Alexander Stein @ 2022-10-17 5:52 UTC (permalink / raw)
To: linux-media, Laurent Pinchart
Cc: Sakari Ailus, Manivannan Sadhasivam, Dave Stevenson
Hello Laurent,
thanks for the updated series.
Am Sonntag, 16. Oktober 2022, 08:15:20 CEST schrieb Laurent Pinchart:
> Registers 0x3012, 0x3013 and 0x3480 are not documented and are set in
> the per-mode register arrays with values indentical for all modes. Move
> them to the common array.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/i2c/imx290.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index 93eab6c96ca0..0b34d60f8ce2 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -192,6 +192,7 @@ static const struct imx290_regval
> imx290_global_init_settings[] = { { IMX290_REG_8BIT(0x300f), 0x00 },
> { IMX290_REG_8BIT(0x3010), 0x21 },
> { IMX290_REG_8BIT(0x3012), 0x64 },
> + { IMX290_REG_8BIT(0x3013), 0x00 },
> { IMX290_REG_8BIT(0x3016), 0x09 },
> { IMX290_REG_8BIT(0x3070), 0x02 },
> { IMX290_REG_8BIT(0x3071), 0x11 },
> @@ -230,6 +231,7 @@ static const struct imx290_regval
> imx290_global_init_settings[] = { { IMX290_REG_8BIT(0x33b0), 0x50 },
> { IMX290_REG_8BIT(0x33b2), 0x1a },
> { IMX290_REG_8BIT(0x33b3), 0x04 },
> + { IMX290_REG_8BIT(0x3480), 0x49 },
> };
>
> static const struct imx290_regval imx290_1080p_settings[] = {
> @@ -239,15 +241,12 @@ static const struct imx290_regval
> imx290_1080p_settings[] = { { IMX290_OPB_SIZE_V, 10 },
> { IMX290_X_OUT_SIZE, 1920 },
> { IMX290_Y_OUT_SIZE, 1080 },
> - { IMX290_REG_8BIT(0x3012), 0x64 },
> - { IMX290_REG_8BIT(0x3013), 0x00 },
> { IMX290_INCKSEL1, 0x18 },
> { IMX290_INCKSEL2, 0x03 },
> { IMX290_INCKSEL3, 0x20 },
> { IMX290_INCKSEL4, 0x01 },
> { IMX290_INCKSEL5, 0x1a },
> { IMX290_INCKSEL6, 0x1a },
> - { IMX290_REG_8BIT(0x3480), 0x49 },
> /* data rate settings */
> { IMX290_REPETITION, 0x10 },
> { IMX290_TCLKPOST, 87 },
> @@ -267,15 +266,12 @@ static const struct imx290_regval
> imx290_720p_settings[] = { { IMX290_OPB_SIZE_V, 4 },
> { IMX290_X_OUT_SIZE, 1280 },
> { IMX290_Y_OUT_SIZE, 720 },
> - { IMX290_REG_8BIT(0x3012), 0x64 },
> - { IMX290_REG_8BIT(0x3013), 0x00 },
> { IMX290_INCKSEL1, 0x20 },
> { IMX290_INCKSEL2, 0x00 },
> { IMX290_INCKSEL3, 0x20 },
> { IMX290_INCKSEL4, 0x01 },
> { IMX290_INCKSEL5, 0x1a },
> { IMX290_INCKSEL6, 0x1a },
> - { IMX290_REG_8BIT(0x3480), 0x49 },
> /* data rate settings */
> { IMX290_REPETITION, 0x10 },
> { IMX290_TCLKPOST, 79 },
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 18/20] media: i2c: imx290: Factor out format retrieval to separate function
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (16 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 17/20] media: i2c: imx290: Move registers with fixed value to init array Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-17 5:55 ` Alexander Stein
2022-10-16 6:15 ` [PATCH v2 19/20] media: i2c: imx290: Add crop selection targets support Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 20/20] media: i2c: imx290: Replace GAIN control with ANALOGUE_GAIN Laurent Pinchart
19 siblings, 1 reply; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
The driver duplicates the same pattern to access the try or active
format in multiple locations. Factor it out to a separate function.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:
- Avoid returning NULL from imx290_get_pad_format()
---
drivers/media/i2c/imx290.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 0b34d60f8ce2..b0ff0e8ed45a 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -519,6 +519,16 @@ static const struct v4l2_ctrl_ops imx290_ctrl_ops = {
.s_ctrl = imx290_set_ctrl,
};
+static struct v4l2_mbus_framefmt *
+imx290_get_pad_format(struct imx290 *imx290, struct v4l2_subdev_state *state,
+ u32 which)
+{
+ if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
+ return &imx290->current_format;
+ else
+ return v4l2_subdev_get_try_format(&imx290->sd, state, 0);
+}
+
static int imx290_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_mbus_code_enum *code)
@@ -562,12 +572,7 @@ static int imx290_get_fmt(struct v4l2_subdev *sd,
mutex_lock(&imx290->lock);
- if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
- framefmt = v4l2_subdev_get_try_format(&imx290->sd, sd_state,
- fmt->pad);
- else
- framefmt = &imx290->current_format;
-
+ framefmt = imx290_get_pad_format(imx290, sd_state, fmt->which);
fmt->format = *framefmt;
mutex_unlock(&imx290->lock);
@@ -627,10 +632,9 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
fmt->format.code = imx290_formats[i].code;
fmt->format.field = V4L2_FIELD_NONE;
- if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
- format = v4l2_subdev_get_try_format(sd, sd_state, fmt->pad);
- } else {
- format = &imx290->current_format;
+ format = imx290_get_pad_format(imx290, sd_state, fmt->which);
+
+ if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
imx290->current_mode = mode;
imx290->bpp = imx290_formats[i].bpp;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 18/20] media: i2c: imx290: Factor out format retrieval to separate function
2022-10-16 6:15 ` [PATCH v2 18/20] media: i2c: imx290: Factor out format retrieval to separate function Laurent Pinchart
@ 2022-10-17 5:55 ` Alexander Stein
2022-10-17 8:37 ` Laurent Pinchart
0 siblings, 1 reply; 29+ messages in thread
From: Alexander Stein @ 2022-10-17 5:55 UTC (permalink / raw)
To: linux-media, Laurent Pinchart
Cc: Sakari Ailus, Manivannan Sadhasivam, Dave Stevenson
Hello Laurent,
thanks for the updated patch.
Am Sonntag, 16. Oktober 2022, 08:15:21 CEST schrieb Laurent Pinchart:
> The driver duplicates the same pattern to access the try or active
> format in multiple locations. Factor it out to a separate function.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v1:
>
> - Avoid returning NULL from imx290_get_pad_format()
> ---
> drivers/media/i2c/imx290.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index 0b34d60f8ce2..b0ff0e8ed45a 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -519,6 +519,16 @@ static const struct v4l2_ctrl_ops imx290_ctrl_ops = {
> .s_ctrl = imx290_set_ctrl,
> };
>
> +static struct v4l2_mbus_framefmt *
> +imx290_get_pad_format(struct imx290 *imx290, struct v4l2_subdev_state
> *state, + u32 which)
> +{
> + if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
> + return &imx290->current_format;
> + else
> + return v4l2_subdev_get_try_format(&imx290->sd, state,
0);
> +}
> +
v4l2_subdev_get_try_format can return NULL, which would be dereferenced later
on. But this happens only if state is NULL itself, which will raise a WARN_ON
anyway. So i guess this is fine.
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> static int imx290_enum_mbus_code(struct v4l2_subdev *sd,
> struct v4l2_subdev_state
*sd_state,
> struct v4l2_subdev_mbus_code_enum
*code)
> @@ -562,12 +572,7 @@ static int imx290_get_fmt(struct v4l2_subdev *sd,
>
> mutex_lock(&imx290->lock);
>
> - if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> - framefmt = v4l2_subdev_get_try_format(&imx290->sd,
sd_state,
> - fmt-
>pad);
> - else
> - framefmt = &imx290->current_format;
> -
> + framefmt = imx290_get_pad_format(imx290, sd_state, fmt->which);
> fmt->format = *framefmt;
>
> mutex_unlock(&imx290->lock);
> @@ -627,10 +632,9 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
> fmt->format.code = imx290_formats[i].code;
> fmt->format.field = V4L2_FIELD_NONE;
>
> - if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> - format = v4l2_subdev_get_try_format(sd, sd_state, fmt-
>pad);
> - } else {
> - format = &imx290->current_format;
> + format = imx290_get_pad_format(imx290, sd_state, fmt->which);
> +
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> imx290->current_mode = mode;
> imx290->bpp = imx290_formats[i].bpp;
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v2 18/20] media: i2c: imx290: Factor out format retrieval to separate function
2022-10-17 5:55 ` Alexander Stein
@ 2022-10-17 8:37 ` Laurent Pinchart
0 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-17 8:37 UTC (permalink / raw)
To: Alexander Stein
Cc: linux-media, Sakari Ailus, Manivannan Sadhasivam, Dave Stevenson
Hi Alexander,
On Mon, Oct 17, 2022 at 07:55:28AM +0200, Alexander Stein wrote:
> Am Sonntag, 16. Oktober 2022, 08:15:21 CEST schrieb Laurent Pinchart:
> > The driver duplicates the same pattern to access the try or active
> > format in multiple locations. Factor it out to a separate function.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v1:
> >
> > - Avoid returning NULL from imx290_get_pad_format()
> > ---
> > drivers/media/i2c/imx290.c | 24 ++++++++++++++----------
> > 1 file changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > index 0b34d60f8ce2..b0ff0e8ed45a 100644
> > --- a/drivers/media/i2c/imx290.c
> > +++ b/drivers/media/i2c/imx290.c
> > @@ -519,6 +519,16 @@ static const struct v4l2_ctrl_ops imx290_ctrl_ops = {
> > .s_ctrl = imx290_set_ctrl,
> > };
> >
> > +static struct v4l2_mbus_framefmt *
> > +imx290_get_pad_format(struct imx290 *imx290, struct v4l2_subdev_state *state,
> > + u32 which)
> > +{
> > + if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
> > + return &imx290->current_format;
> > + else
> > + return v4l2_subdev_get_try_format(&imx290->sd, state, 0);
> > +}
> > +
>
> v4l2_subdev_get_try_format can return NULL, which would be dereferenced later
> on. But this happens only if state is NULL itself, which will raise a WARN_ON
> anyway. So i guess this is fine.
I'll add a patch on top of the series to convert the driver to the V4L2
active state, that should simplify all this.
> Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>
> > static int imx290_enum_mbus_code(struct v4l2_subdev *sd,
> > struct v4l2_subdev_state *sd_state,
> > struct v4l2_subdev_mbus_code_enum *code)
> > @@ -562,12 +572,7 @@ static int imx290_get_fmt(struct v4l2_subdev *sd,
> >
> > mutex_lock(&imx290->lock);
> >
> > - if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > - framefmt = v4l2_subdev_get_try_format(&imx290->sd, sd_state,
> > - fmt->pad);
> > - else
> > - framefmt = &imx290->current_format;
> > -
> > + framefmt = imx290_get_pad_format(imx290, sd_state, fmt->which);
> > fmt->format = *framefmt;
> >
> > mutex_unlock(&imx290->lock);
> > @@ -627,10 +632,9 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
> > fmt->format.code = imx290_formats[i].code;
> > fmt->format.field = V4L2_FIELD_NONE;
> >
> > - if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
> > - format = v4l2_subdev_get_try_format(sd, sd_state, fmt->pad);
> > - } else {
> > - format = &imx290->current_format;
> > + format = imx290_get_pad_format(imx290, sd_state, fmt->which);
> > +
> > + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> > imx290->current_mode = mode;
> > imx290->bpp = imx290_formats[i].bpp;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 19/20] media: i2c: imx290: Add crop selection targets support
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (17 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 18/20] media: i2c: imx290: Factor out format retrieval to separate function Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
2022-10-16 6:15 ` [PATCH v2 20/20] media: i2c: imx290: Replace GAIN control with ANALOGUE_GAIN Laurent Pinchart
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
Implement read-only access to crop selection rectangles to expose the
analogue crop rectangle. The public (leaked) IMX290 documentation is not
very clear on how cropping is implemented and configured exactly, so
the margins may not be entirely accurate.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:
- Fix effective bottom margin value in diagram
---
drivers/media/i2c/imx290.c | 94 ++++++++++++++++++++++++++++++++++++++
1 file changed, 94 insertions(+)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index b0ff0e8ed45a..53db66068538 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -105,6 +105,53 @@
#define IMX290_VMAX_DEFAULT 1125
+
+/*
+ * The IMX290 pixel array is organized as follows:
+ *
+ * +------------------------------------+
+ * | Optical Black | } Vertical effective optical black (10)
+ * +---+------------------------------------+---+
+ * | | | | } Effective top margin (8)
+ * | | +----------------------------+ | | \
+ * | | | | | | |
+ * | | | | | | |
+ * | | | | | | |
+ * | | | Recording Pixel Area | | | | Recommended height (1080)
+ * | | | | | | |
+ * | | | | | | |
+ * | | | | | | |
+ * | | +----------------------------+ | | /
+ * | | | | } Effective bottom margin (9)
+ * +---+------------------------------------+---+
+ * <-> <-> <--------------------------> <-> <->
+ * \---- Ignored right margin (4)
+ * \-------- Effective right margin (9)
+ * \------------------------- Recommended width (1920)
+ * \----------------------------------------- Effective left margin (8)
+ * \--------------------------------------------- Ignored left margin (4)
+ *
+ * The optical black lines are output over CSI-2 with a separate data type.
+ *
+ * The pixel array is meant to have 1920x1080 usable pixels after image
+ * processing in an ISP. It has 8 (9) extra active pixels usable for color
+ * processing in the ISP on the top and left (bottom and right) sides of the
+ * image. In addition, 4 additional pixels are present on the left and right
+ * sides of the image, documented as "ignored area".
+ *
+ * As far as is understood, all pixels of the pixel array (ignored area, color
+ * processing margins and recording area) can be output by the sensor.
+ */
+
+#define IMX290_PIXEL_ARRAY_WIDTH 1945
+#define IMX290_PIXEL_ARRAY_HEIGHT 1097
+#define IMX920_PIXEL_ARRAY_MARGIN_LEFT 12
+#define IMX920_PIXEL_ARRAY_MARGIN_RIGHT 13
+#define IMX920_PIXEL_ARRAY_MARGIN_TOP 8
+#define IMX920_PIXEL_ARRAY_MARGIN_BOTTOM 9
+#define IMX290_PIXEL_ARRAY_RECORDING_WIDTH 1920
+#define IMX290_PIXEL_ARRAY_RECORDING_HEIGHT 1080
+
static const char * const imx290_supply_name[] = {
"vdda",
"vddd",
@@ -667,6 +714,52 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
return 0;
}
+static int imx290_get_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_selection *sel)
+{
+ struct imx290 *imx290 = to_imx290(sd);
+ struct v4l2_mbus_framefmt *format;
+
+ switch (sel->target) {
+ case V4L2_SEL_TGT_CROP: {
+ format = imx290_get_pad_format(imx290, sd_state, sel->which);
+
+ mutex_lock(&imx290->lock);
+
+ sel->r.top = IMX920_PIXEL_ARRAY_MARGIN_TOP
+ + (IMX290_PIXEL_ARRAY_RECORDING_HEIGHT - format->height) / 2;
+ sel->r.left = IMX920_PIXEL_ARRAY_MARGIN_LEFT
+ + (IMX290_PIXEL_ARRAY_RECORDING_WIDTH - format->width) / 2;
+ sel->r.width = format->width;
+ sel->r.height = format->height;
+
+ mutex_unlock(&imx290->lock);
+ return 0;
+ }
+
+ case V4L2_SEL_TGT_NATIVE_SIZE:
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ sel->r.top = 0;
+ sel->r.left = 0;
+ sel->r.width = IMX290_PIXEL_ARRAY_WIDTH;
+ sel->r.height = IMX290_PIXEL_ARRAY_HEIGHT;
+
+ return 0;
+
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ sel->r.top = IMX920_PIXEL_ARRAY_MARGIN_TOP;
+ sel->r.left = IMX920_PIXEL_ARRAY_MARGIN_LEFT;
+ sel->r.width = IMX290_PIXEL_ARRAY_RECORDING_WIDTH;
+ sel->r.height = IMX290_PIXEL_ARRAY_RECORDING_HEIGHT;
+
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+}
+
static int imx290_entity_init_cfg(struct v4l2_subdev *subdev,
struct v4l2_subdev_state *sd_state)
{
@@ -883,6 +976,7 @@ static const struct v4l2_subdev_pad_ops imx290_pad_ops = {
.enum_frame_size = imx290_enum_frame_size,
.get_fmt = imx290_get_fmt,
.set_fmt = imx290_set_fmt,
+ .get_selection = imx290_get_selection,
};
static const struct v4l2_subdev_ops imx290_subdev_ops = {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 20/20] media: i2c: imx290: Replace GAIN control with ANALOGUE_GAIN
2022-10-16 6:15 [PATCH v2 00/20] media: i2c: imx290: Miscellaneous improvements Laurent Pinchart
` (18 preceding siblings ...)
2022-10-16 6:15 ` [PATCH v2 19/20] media: i2c: imx290: Add crop selection targets support Laurent Pinchart
@ 2022-10-16 6:15 ` Laurent Pinchart
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Pinchart @ 2022-10-16 6:15 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Manivannan Sadhasivam, Alexander Stein,
Dave Stevenson
The IMX290 gain register controls the analogue gain. Replace the
V4L2_CID_GAIN control with V4L2_CID_ANALOGUE_GAIN.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/media/i2c/imx290.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 53db66068538..ffc072535be0 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -524,7 +524,7 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
return 0;
switch (ctrl->id) {
- case V4L2_CID_GAIN:
+ case V4L2_CID_ANALOGUE_GAIN:
ret = imx290_write(imx290, IMX290_GAIN, ctrl->val, NULL);
break;
@@ -1015,7 +1015,7 @@ static int imx290_ctrl_init(struct imx290 *imx290)
* gain control should be adjusted accordingly.
*/
v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
- V4L2_CID_GAIN, 0, 100, 1, 0);
+ V4L2_CID_ANALOGUE_GAIN, 0, 100, 1, 0);
v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
V4L2_CID_EXPOSURE, 1, IMX290_VMAX_DEFAULT - 2, 1,
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 29+ messages in thread