2023-08-11 11:56:31

by Jai Luthra

[permalink] [raw]
Subject: [PATCH v9 00/13] CSI2RX support on J721E and AM62

From: Pratyush Yadav <[email protected]>

Hi,

This series adds support for CSI2 capture on J721E. It includes some
fixes to the Cadence CSI2RX driver, and adds the TI CSI2RX wrapper driver.

This is the v9 of the below v8 series,
https://lore.kernel.org/r/[email protected]

Testing logs: https://gist.github.com/jailuthra/eaeb3af3c65b67e1bc0d5db28180131d

J721E CSI2RX driver can also be extended to support multi-stream
capture, filtering different CSI Virtual Channels (VC) or Data Types
(DT) to different DMA channels. A WIP series based on v7 is available
for reference at https://github.com/jailuthra/linux/commits/csi_multi_wip

I will rebase the multi-stream patches on the current series (v9) and
post them as RFC in the coming weeks.

Signed-off-by: Jai Luthra <[email protected]>
---

Changelog from v8
=================

Range-diff: https://0x0.st/H_xh.diff

Dropped the following patches:
[v8 01/16] media: subdev: Export get_format helper for link validation
- Using subdev's get_fmt directly instead
[v8 04/16] media: cadence: Add support for TI SoCs
- Don't add a compatible if we are not using it in the driver
[v8 14/16] media: cadence: csi2rx: Support RAW8 and RAW10 formats
- Squashed into a previous patch [v8 07/16]

For [05/13] media: cadence: csi2rx: Add get_fmt and set_fmt pad ops:
- Squash the patch adding RAW8 and RAW10 formats within this one
- Single line struct entries in formats[] array
- Skip specifiying redundant format.which entry in init_cfg()

For [06/13] media: cadence: csi2rx: Configure DPHY using link freq:
- Don't specify stream while calling .get_fmt()

For [07/13] media: cadence: csi2rx: Soft reset the streams before starting capture:
- Simplify reset sequence, minimizing delays

For [08/13] media: cadence: csi2rx: Set the STOP bit when stopping a stream:
- Better log message to avoid confusion between cadence streams and v4l2
streams

For [13/13] media: ti: Add CSI2RX support for J721E:
- Allocate drain buffer at start of stream instead of doing it in the
middle, and document why it is needed in comments
- Call subdev's get_fmt directly for link_validation()
- Cleanup height/width clamping and rounding code, document it in comments
- Return and check errors from setup_shim()
- s/subdev/source for cadence csi2rx's v4l2_subdev
- s/ti_csi2rx_init_subdev/ti_csi2rx_notifier_register
- Change copyright year/author list

---
Jai Luthra (1):
media: dt-bindings: cadence-csi2rx: Add TI compatible string

Pratyush Yadav (12):
media: dt-bindings: Make sure items in data-lanes are unique
media: cadence: csi2rx: Unregister v4l2 async notifier
media: cadence: csi2rx: Cleanup media entity properly
media: cadence: csi2rx: Add get_fmt and set_fmt pad ops
media: cadence: csi2rx: Configure DPHY using link freq
media: cadence: csi2rx: Soft reset the streams before starting capture
media: cadence: csi2rx: Set the STOP bit when stopping a stream
media: cadence: csi2rx: Fix stream data configuration
media: cadence: csi2rx: Populate subdev devnode
media: cadence: csi2rx: Add link validation
media: dt-bindings: Add TI J721E CSI2RX
media: ti: Add CSI2RX support for J721E

.../devicetree/bindings/media/cdns,csi2rx.yaml | 1 +
.../bindings/media/ti,j721e-csi2rx-shim.yaml | 100 ++
.../bindings/media/video-interfaces.yaml | 1 +
MAINTAINERS | 7 +
drivers/media/platform/cadence/cdns-csi2rx.c | 181 ++-
drivers/media/platform/ti/Kconfig | 12 +
drivers/media/platform/ti/Makefile | 1 +
drivers/media/platform/ti/j721e-csi2rx/Makefile | 2 +
.../media/platform/ti/j721e-csi2rx/j721e-csi2rx.c | 1150 ++++++++++++++++++++
9 files changed, 1448 insertions(+), 7 deletions(-)
---
base-commit: 21ef7b1e17d039053edaeaf41142423810572741
change-id: 20230727-upstream_csi-acbeabe038d8

Best regards,
--
Jai Luthra <[email protected]>


2023-08-11 12:07:53

by Jai Luthra

[permalink] [raw]
Subject: [PATCH v9 04/13] media: cadence: csi2rx: Cleanup media entity properly

From: Pratyush Yadav <[email protected]>

Call media_entity_cleanup() in probe error path and remove to make sure
the media entity is cleaned up properly.

Suggested-by: Laurent Pinchart <[email protected]>
Signed-off-by: Pratyush Yadav <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Tomi Valkeinen <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Jai Luthra <[email protected]>
---
drivers/media/platform/cadence/cdns-csi2rx.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 9231ee7e9b3a..9de3240e261c 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -547,6 +547,7 @@ static int csi2rx_probe(struct platform_device *pdev)
err_cleanup:
v4l2_async_nf_unregister(&csi2rx->notifier);
v4l2_async_nf_cleanup(&csi2rx->notifier);
+ media_entity_cleanup(&csi2rx->subdev.entity);
err_free_priv:
kfree(csi2rx);
return ret;
@@ -559,6 +560,7 @@ static void csi2rx_remove(struct platform_device *pdev)
v4l2_async_nf_unregister(&csi2rx->notifier);
v4l2_async_nf_cleanup(&csi2rx->notifier);
v4l2_async_unregister_subdev(&csi2rx->subdev);
+ media_entity_cleanup(&csi2rx->subdev.entity);
kfree(csi2rx);
}


--
2.41.0

2023-08-11 12:12:05

by Jai Luthra

[permalink] [raw]
Subject: [PATCH v9 10/13] media: cadence: csi2rx: Populate subdev devnode

From: Pratyush Yadav <[email protected]>

The devnode can be used by media-ctl and other userspace tools to
perform configurations on the subdev. Without it, media-ctl returns
ENOENT when setting format on the sensor subdev.

Signed-off-by: Pratyush Yadav <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Tomi Valkeinen <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Jai Luthra <[email protected]>
---
drivers/media/platform/cadence/cdns-csi2rx.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 46effbbe580d..0947b112a573 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -673,6 +673,7 @@ static int csi2rx_probe(struct platform_device *pdev)
csi2rx->pads[CSI2RX_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
for (i = CSI2RX_PAD_SOURCE_STREAM0; i < CSI2RX_PAD_MAX; i++)
csi2rx->pads[i].flags = MEDIA_PAD_FL_SOURCE;
+ csi2rx->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;

ret = media_entity_pads_init(&csi2rx->subdev.entity, CSI2RX_PAD_MAX,
csi2rx->pads);

--
2.41.0

2023-08-11 12:32:13

by Jai Luthra

[permalink] [raw]
Subject: [PATCH v9 09/13] media: cadence: csi2rx: Fix stream data configuration

From: Pratyush Yadav <[email protected]>

Firstly, there is no VC_EN bit present in the STREAM_DATA_CFG register.
Bit 31 is part of the VL_SELECT field. Remove it completely.

Secondly, it makes little sense to enable ith virtual channel for ith
stream. Sure, there might be a use-case that demands it. But there might
also be a use case that demands all streams to use the 0th virtual
channel. Prefer this case over the former because it is less arbitrary
and also makes it very clear what the limitations of the current driver
is instead of giving a false impression that multiple virtual channels
are supported.

Signed-off-by: Pratyush Yadav <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Tomi Valkeinen <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Jai Luthra <[email protected]>
---
drivers/media/platform/cadence/cdns-csi2rx.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index f8205c3a28c0..46effbbe580d 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -49,7 +49,6 @@
#define CSI2RX_STREAM_STATUS_RDY BIT(31)

#define CSI2RX_STREAM_DATA_CFG_REG(n) (CSI2RX_STREAM_BASE(n) + 0x008)
-#define CSI2RX_STREAM_DATA_CFG_EN_VC_SELECT BIT(31)
#define CSI2RX_STREAM_DATA_CFG_VC_SELECT(n) BIT((n) + 16)

#define CSI2RX_STREAM_CFG_REG(n) (CSI2RX_STREAM_BASE(n) + 0x00c)
@@ -271,8 +270,11 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
writel(CSI2RX_STREAM_CFG_FIFO_MODE_LARGE_BUF,
csi2rx->base + CSI2RX_STREAM_CFG_REG(i));

- writel(CSI2RX_STREAM_DATA_CFG_EN_VC_SELECT |
- CSI2RX_STREAM_DATA_CFG_VC_SELECT(i),
+ /*
+ * Enable one virtual channel. When multiple virtual channels
+ * are supported this will have to be changed.
+ */
+ writel(CSI2RX_STREAM_DATA_CFG_VC_SELECT(0),
csi2rx->base + CSI2RX_STREAM_DATA_CFG_REG(i));

writel(CSI2RX_STREAM_CTRL_START,

--
2.41.0

2023-08-11 12:42:25

by Jai Luthra

[permalink] [raw]
Subject: [PATCH v9 11/13] media: cadence: csi2rx: Add link validation

From: Pratyush Yadav <[email protected]>

Add media link validation to make sure incorrectly configured pipelines
are caught.

Signed-off-by: Pratyush Yadav <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Tomi Valkeinen <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Jai Luthra <[email protected]>
---
drivers/media/platform/cadence/cdns-csi2rx.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 0947b112a573..5eeeb398cdb5 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -458,6 +458,10 @@ static const struct v4l2_subdev_ops csi2rx_subdev_ops = {
.pad = &csi2rx_pad_ops,
};

+static const struct media_entity_operations csi2rx_media_ops = {
+ .link_validate = v4l2_subdev_link_validate,
+};
+
static int csi2rx_async_bound(struct v4l2_async_notifier *notifier,
struct v4l2_subdev *s_subdev,
struct v4l2_async_connection *asd)
@@ -674,6 +678,7 @@ static int csi2rx_probe(struct platform_device *pdev)
for (i = CSI2RX_PAD_SOURCE_STREAM0; i < CSI2RX_PAD_MAX; i++)
csi2rx->pads[i].flags = MEDIA_PAD_FL_SOURCE;
csi2rx->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ csi2rx->subdev.entity.ops = &csi2rx_media_ops;

ret = media_entity_pads_init(&csi2rx->subdev.entity, CSI2RX_PAD_MAX,
csi2rx->pads);

--
2.41.0

2023-08-11 12:47:05

by Jai Luthra

[permalink] [raw]
Subject: [PATCH v9 12/13] media: dt-bindings: Add TI J721E CSI2RX

From: Pratyush Yadav <[email protected]>

TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
parts together.

Signed-off-by: Pratyush Yadav <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Jai Luthra <[email protected]>
---
NOTE:

This patch depends on
9536cc949235 ("media: dt-bindings: cadence-csi2rx: Convert to DT schema")
which is part of linux-next.

.../bindings/media/ti,j721e-csi2rx-shim.yaml | 100 +++++++++++++++++++++
1 file changed, 100 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.yaml b/Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.yaml
new file mode 100644
index 000000000000..f762fdc05e4d
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/ti,j721e-csi2rx-shim.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI J721E CSI2RX Shim
+
+description: |
+ The TI J721E CSI2RX Shim is a wrapper around Cadence CSI2RX bridge that
+ enables sending captured frames to memory over PSI-L DMA. In the J721E
+ Technical Reference Manual (SPRUIL1B) it is referred to as "SHIM" under the
+ CSI_RX_IF section.
+
+maintainers:
+ - Jai Luthra <[email protected]>
+
+properties:
+ compatible:
+ const: ti,j721e-csi2rx-shim
+
+ dmas:
+ maxItems: 1
+
+ dma-names:
+ items:
+ - const: rx0
+
+ reg:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 1
+
+ ranges: true
+
+ "#address-cells": true
+
+ "#size-cells": true
+
+patternProperties:
+ "^csi-bridge@":
+ type: object
+ description: CSI2 bridge node.
+ $ref: cdns,csi2rx.yaml#
+
+required:
+ - compatible
+ - reg
+ - dmas
+ - dma-names
+ - power-domains
+ - ranges
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/soc/ti,sci_pm_domain.h>
+
+ ti_csi2rx0: ticsi2rx@4500000 {
+ compatible = "ti,j721e-csi2rx-shim";
+ dmas = <&main_udmap 0x4940>;
+ dma-names = "rx0";
+ reg = <0x4500000 0x1000>;
+ power-domains = <&k3_pds 26 TI_SCI_PD_EXCLUSIVE>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ cdns_csi2rx: csi-bridge@4504000 {
+ compatible = "ti,j721e-csi2rx", "cdns,csi2rx";
+ reg = <0x4504000 0x1000>;
+ clocks = <&k3_clks 26 2>, <&k3_clks 26 0>, <&k3_clks 26 2>,
+ <&k3_clks 26 2>, <&k3_clks 26 3>, <&k3_clks 26 3>;
+ clock-names = "sys_clk", "p_clk", "pixel_if0_clk",
+ "pixel_if1_clk", "pixel_if2_clk", "pixel_if3_clk";
+ phys = <&dphy0>;
+ phy-names = "dphy";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ csi2_0: port@0 {
+
+ reg = <0>;
+
+ csi2rx0_in_sensor: endpoint {
+ remote-endpoint = <&csi2_cam0>;
+ bus-type = <4>; /* CSI2 DPHY. */
+ clock-lanes = <0>;
+ data-lanes = <1 2>;
+ };
+ };
+ };
+ };
+ };

--
2.41.0

2023-08-11 12:48:01

by Jai Luthra

[permalink] [raw]
Subject: [PATCH v9 03/13] media: cadence: csi2rx: Unregister v4l2 async notifier

From: Pratyush Yadav <[email protected]>

The notifier is added to the global notifier list when registered. When
the module is removed, the struct csi2rx_priv in which the notifier is
embedded, is destroyed. As a result the notifier list has a reference to
a notifier that no longer exists. This causes invalid memory accesses
when the list is iterated over. Similar for when the probe fails.
Unregister and clean up the notifier to avoid this.

Fixes: 1fc3b37f34f6 ("media: v4l: cadence: Add Cadence MIPI-CSI2 RX driver")

Signed-off-by: Pratyush Yadav <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Tomi Valkeinen <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Jai Luthra <[email protected]>
---
drivers/media/platform/cadence/cdns-csi2rx.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 0d879d71d818..9231ee7e9b3a 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -479,8 +479,10 @@ static int csi2rx_parse_dt(struct csi2rx_priv *csi2rx)
asd = v4l2_async_nf_add_fwnode_remote(&csi2rx->notifier, fwh,
struct v4l2_async_connection);
of_node_put(ep);
- if (IS_ERR(asd))
+ if (IS_ERR(asd)) {
+ v4l2_async_nf_cleanup(&csi2rx->notifier);
return PTR_ERR(asd);
+ }

csi2rx->notifier.ops = &csi2rx_notifier_ops;

@@ -543,6 +545,7 @@ static int csi2rx_probe(struct platform_device *pdev)
return 0;

err_cleanup:
+ v4l2_async_nf_unregister(&csi2rx->notifier);
v4l2_async_nf_cleanup(&csi2rx->notifier);
err_free_priv:
kfree(csi2rx);
@@ -553,6 +556,8 @@ static void csi2rx_remove(struct platform_device *pdev)
{
struct csi2rx_priv *csi2rx = platform_get_drvdata(pdev);

+ v4l2_async_nf_unregister(&csi2rx->notifier);
+ v4l2_async_nf_cleanup(&csi2rx->notifier);
v4l2_async_unregister_subdev(&csi2rx->subdev);
kfree(csi2rx);
}

--
2.41.0

2023-08-11 12:50:55

by Jai Luthra

[permalink] [raw]
Subject: [PATCH v9 06/13] media: cadence: csi2rx: Configure DPHY using link freq

From: Pratyush Yadav <[email protected]>

Some platforms like TI's J721E can have the CSI2RX paired with an
external DPHY. Use the generic PHY framework to configure the DPHY with
the correct link frequency.

Signed-off-by: Pratyush Yadav <[email protected]>
Co-authored-by: Jai Luthra <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Tomi Valkeinen <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Jai Luthra <[email protected]>
---
Changes from v8:
- Don't specify stream while calling .get_fmt()

drivers/media/platform/cadence/cdns-csi2rx.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 047e74ee2443..933edec89520 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -145,8 +145,32 @@ static void csi2rx_reset(struct csi2rx_priv *csi2rx)
static int csi2rx_configure_ext_dphy(struct csi2rx_priv *csi2rx)
{
union phy_configure_opts opts = { };
+ struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy;
+ struct v4l2_subdev_format sd_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ .pad = CSI2RX_PAD_SINK,
+ };
+ const struct csi2rx_fmt *fmt;
+ s64 link_freq;
int ret;

+ ret = v4l2_subdev_call_state_active(&csi2rx->subdev, pad, get_fmt,
+ &sd_fmt);
+ if (ret < 0)
+ return ret;
+
+ fmt = csi2rx_get_fmt_by_code(sd_fmt.format.code);
+
+ link_freq = v4l2_get_link_freq(csi2rx->source_subdev->ctrl_handler,
+ fmt->bpp, 2 * csi2rx->num_lanes);
+ if (link_freq < 0)
+ return link_freq;
+
+ ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq,
+ csi2rx->num_lanes, cfg);
+ if (ret)
+ return ret;
+
ret = phy_power_on(csi2rx->dphy);
if (ret)
return ret;

--
2.41.0

2023-08-11 13:21:24

by Jai Luthra

[permalink] [raw]
Subject: [PATCH v9 01/13] media: dt-bindings: Make sure items in data-lanes are unique

From: Pratyush Yadav <[email protected]>

The data-lanes property maps the logical lane numbers to the physical
lane numbers. The position of an entry is the logical lane number and
its value is the physical lane number. Since one physical lane can only
map to one logical lane, no number in the list should repeat. Add the
uniqueItems constraint on the property to enforce this.

Signed-off-by: Pratyush Yadav <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Tomi Valkeinen <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Jai Luthra <[email protected]>
---
Documentation/devicetree/bindings/media/video-interfaces.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.yaml b/Documentation/devicetree/bindings/media/video-interfaces.yaml
index a211d49dc2ac..26e3e7d7c67b 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.yaml
+++ b/Documentation/devicetree/bindings/media/video-interfaces.yaml
@@ -160,6 +160,7 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint32-array
minItems: 1
maxItems: 8
+ uniqueItems: true
items:
# Assume up to 9 physical lane indices
maximum: 8

--
2.41.0

2023-08-11 13:36:02

by Jai Luthra

[permalink] [raw]
Subject: [PATCH v9 05/13] media: cadence: csi2rx: Add get_fmt and set_fmt pad ops

From: Pratyush Yadav <[email protected]>

The format is needed to calculate the link speed for the external DPHY
configuration. It is not right to query the format from the source
subdev. Add get_fmt and set_fmt pad operations so that the format can be
configured and correct bpp be selected.

Initialize and use the v4l2 subdev active state to keep track of the
active formats. Also propagate the new format from the sink pad to all
the source pads.

Signed-off-by: Pratyush Yadav <[email protected]>
Co-authored-by: Jai Luthra <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Jai Luthra <[email protected]>
---
Changes from v8:
- Squash the patch adding RAW8 and RAW10 formats within this one
- Single line struct entries in formats[] array
- Skip specifiying redundant format.which entry in init_cfg()

drivers/media/platform/cadence/cdns-csi2rx.c | 101 ++++++++++++++++++++++++++-
1 file changed, 100 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 9de3240e261c..047e74ee2443 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -61,6 +61,11 @@ enum csi2rx_pads {
CSI2RX_PAD_MAX,
};

+struct csi2rx_fmt {
+ u32 code;
+ u8 bpp;
+};
+
struct csi2rx_priv {
struct device *dev;
unsigned int count;
@@ -95,6 +100,32 @@ struct csi2rx_priv {
int source_pad;
};

+static const struct csi2rx_fmt formats[] = {
+ { .code = MEDIA_BUS_FMT_YUYV8_1X16, .bpp = 16, },
+ { .code = MEDIA_BUS_FMT_UYVY8_1X16, .bpp = 16, },
+ { .code = MEDIA_BUS_FMT_YVYU8_1X16, .bpp = 16, },
+ { .code = MEDIA_BUS_FMT_VYUY8_1X16, .bpp = 16, },
+ { .code = MEDIA_BUS_FMT_SBGGR8_1X8, .bpp = 8, },
+ { .code = MEDIA_BUS_FMT_SGBRG8_1X8, .bpp = 8, },
+ { .code = MEDIA_BUS_FMT_SGRBG8_1X8, .bpp = 8, },
+ { .code = MEDIA_BUS_FMT_SRGGB8_1X8, .bpp = 8, },
+ { .code = MEDIA_BUS_FMT_SBGGR10_1X10, .bpp = 10, },
+ { .code = MEDIA_BUS_FMT_SGBRG10_1X10, .bpp = 10, },
+ { .code = MEDIA_BUS_FMT_SGRBG10_1X10, .bpp = 10, },
+ { .code = MEDIA_BUS_FMT_SRGGB10_1X10, .bpp = 10, },
+};
+
+static const struct csi2rx_fmt *csi2rx_get_fmt_by_code(u32 code)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(formats); i++)
+ if (formats[i].code == code)
+ return &formats[i];
+
+ return NULL;
+}
+
static inline
struct csi2rx_priv *v4l2_subdev_to_csi2rx(struct v4l2_subdev *subdev)
{
@@ -303,12 +334,73 @@ static int csi2rx_s_stream(struct v4l2_subdev *subdev, int enable)
return ret;
}

+static int csi2rx_set_fmt(struct v4l2_subdev *subdev,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_format *format)
+{
+ struct v4l2_mbus_framefmt *fmt;
+ unsigned int i;
+
+ /* No transcoding, source and sink formats must match. */
+ if (format->pad != CSI2RX_PAD_SINK)
+ return v4l2_subdev_get_fmt(subdev, state, format);
+
+ if (!csi2rx_get_fmt_by_code(format->format.code))
+ format->format.code = formats[0].code;
+
+ format->format.field = V4L2_FIELD_NONE;
+
+ /* Set sink format */
+ fmt = v4l2_subdev_get_pad_format(subdev, state, format->pad);
+ if (!fmt)
+ return -EINVAL;
+
+ *fmt = format->format;
+
+ /* Propagate to source formats */
+ for (i = CSI2RX_PAD_SOURCE_STREAM0; i < CSI2RX_PAD_MAX; i++) {
+ fmt = v4l2_subdev_get_pad_format(subdev, state, i);
+ if (!fmt)
+ return -EINVAL;
+ *fmt = format->format;
+ }
+
+ return 0;
+}
+
+static int csi2rx_init_cfg(struct v4l2_subdev *subdev,
+ struct v4l2_subdev_state *state)
+{
+ struct v4l2_subdev_format format = {
+ .pad = CSI2RX_PAD_SINK,
+ .format = {
+ .width = 640,
+ .height = 480,
+ .code = MEDIA_BUS_FMT_UYVY8_1X16,
+ .field = V4L2_FIELD_NONE,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .ycbcr_enc = V4L2_YCBCR_ENC_601,
+ .quantization = V4L2_QUANTIZATION_LIM_RANGE,
+ .xfer_func = V4L2_XFER_FUNC_SRGB,
+ },
+ };
+
+ return csi2rx_set_fmt(subdev, state, &format);
+}
+
+static const struct v4l2_subdev_pad_ops csi2rx_pad_ops = {
+ .get_fmt = v4l2_subdev_get_fmt,
+ .set_fmt = csi2rx_set_fmt,
+ .init_cfg = csi2rx_init_cfg,
+};
+
static const struct v4l2_subdev_video_ops csi2rx_video_ops = {
.s_stream = csi2rx_s_stream,
};

static const struct v4l2_subdev_ops csi2rx_subdev_ops = {
.video = &csi2rx_video_ops,
+ .pad = &csi2rx_pad_ops,
};

static int csi2rx_async_bound(struct v4l2_async_notifier *notifier,
@@ -532,9 +624,13 @@ static int csi2rx_probe(struct platform_device *pdev)
if (ret)
goto err_cleanup;

+ ret = v4l2_subdev_init_finalize(&csi2rx->subdev);
+ if (ret)
+ goto err_cleanup;
+
ret = v4l2_async_register_subdev(&csi2rx->subdev);
if (ret < 0)
- goto err_cleanup;
+ goto err_free_state;

dev_info(&pdev->dev,
"Probed CSI2RX with %u/%u lanes, %u streams, %s D-PHY\n",
@@ -544,6 +640,8 @@ static int csi2rx_probe(struct platform_device *pdev)

return 0;

+err_free_state:
+ v4l2_subdev_cleanup(&csi2rx->subdev);
err_cleanup:
v4l2_async_nf_unregister(&csi2rx->notifier);
v4l2_async_nf_cleanup(&csi2rx->notifier);
@@ -560,6 +658,7 @@ static void csi2rx_remove(struct platform_device *pdev)
v4l2_async_nf_unregister(&csi2rx->notifier);
v4l2_async_nf_cleanup(&csi2rx->notifier);
v4l2_async_unregister_subdev(&csi2rx->subdev);
+ v4l2_subdev_cleanup(&csi2rx->subdev);
media_entity_cleanup(&csi2rx->subdev.entity);
kfree(csi2rx);
}

--
2.41.0

2023-08-11 14:42:50

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v9 12/13] media: dt-bindings: Add TI J721E CSI2RX


On Fri, 11 Aug 2023 16:17:34 +0530, Jai Luthra wrote:
> From: Pratyush Yadav <[email protected]>
>
> TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
> capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
> parts together.
>
> Signed-off-by: Pratyush Yadav <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Signed-off-by: Jai Luthra <[email protected]>
> ---
> NOTE:
>
> This patch depends on
> 9536cc949235 ("media: dt-bindings: cadence-csi2rx: Convert to DT schema")
> which is part of linux-next.
>
> .../bindings/media/ti,j721e-csi2rx-shim.yaml | 100 +++++++++++++++++++++
> 1 file changed, 100 insertions(+)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.yaml:
Error in referenced schema matching $id: http://devicetree.org/schemas/media/cdns,csi2rx.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.example.dtb: ticsi2rx@4500000: csi-bridge@4504000: False schema does not allow {'compatible': ['ti,j721e-csi2rx', 'cdns,csi2rx'], 'reg': [[72368128, 4096]], 'clocks': [[4294967295, 26, 2], [4294967295, 26, 0], [4294967295, 26, 2], [4294967295, 26, 2], [4294967295, 26, 3], [4294967295, 26, 3]], 'clock-names': ['sys_clk', 'p_clk', 'pixel_if0_clk', 'pixel_if1_clk', 'pixel_if2_clk', 'pixel_if3_clk'], 'phys': [[4294967295]], 'phy-names': ['dphy'], 'ports': {'#address-cells': [[1]], '#size-cells': [[0]], 'port@0': {'reg': [[0]], 'endpoint': {'remote-endpoint': [[4294967295]], 'bus-type': [[4]], 'clock-lanes': [[0]], 'data-lanes': [[1, 2]]}}}}
from schema $id: http://devicetree.org/schemas/media/ti,j721e-csi2rx-shim.yaml#
Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.example.dtb: /example-0/ticsi2rx@4500000/csi-bridge@4504000: failed to match any schema with compatible: ['ti,j721e-csi2rx', 'cdns,csi2rx']
Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.example.dtb: /example-0/ticsi2rx@4500000/csi-bridge@4504000: failed to match any schema with compatible: ['ti,j721e-csi2rx', 'cdns,csi2rx']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


2023-08-11 15:34:00

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v9 12/13] media: dt-bindings: Add TI J721E CSI2RX

On Fri, Aug 11, 2023 at 08:00:55AM -0600, Rob Herring wrote:
>
> On Fri, 11 Aug 2023 16:17:34 +0530, Jai Luthra wrote:
> > From: Pratyush Yadav <[email protected]>
> >
> > TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
> > capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
> > parts together.
> >
> > Signed-off-by: Pratyush Yadav <[email protected]>
> > Reviewed-by: Laurent Pinchart <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > Signed-off-by: Jai Luthra <[email protected]>
> > ---
> > NOTE:
> >
> > This patch depends on
> > 9536cc949235 ("media: dt-bindings: cadence-csi2rx: Convert to DT schema")
> > which is part of linux-next.
> >
> > .../bindings/media/ti,j721e-csi2rx-shim.yaml | 100 +++++++++++++++++++++
> > 1 file changed, 100 insertions(+)
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.yaml:
> Error in referenced schema matching $id: http://devicetree.org/schemas/media/cdns,csi2rx.yaml
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.example.dtb: ticsi2rx@4500000: csi-bridge@4504000: False schema does not allow {'compatible': ['ti,j721e-csi2rx', 'cdns,csi2rx'], 'reg': [[72368128, 4096]], 'clocks': [[4294967295, 26, 2], [4294967295, 26, 0], [4294967295, 26, 2], [4294967295, 26, 2], [4294967295, 26, 3], [4294967295, 26, 3]], 'clock-names': ['sys_clk', 'p_clk', 'pixel_if0_clk', 'pixel_if1_clk', 'pixel_if2_clk', 'pixel_if3_clk'], 'phys': [[4294967295]], 'phy-names': ['dphy'], 'ports': {'#address-cells': [[1]], '#size-cells': [[0]], 'port@0': {'reg': [[0]], 'endpoint': {'remote-endpoint': [[4294967295]], 'bus-type': [[4]], 'clock-lanes': [[0]], 'data-lanes': [[1, 2]]}}}}
> from schema $id: http://devicetree.org/schemas/media/ti,j721e-csi2rx-shim.yaml#
> Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.example.dtb: /example-0/ticsi2rx@4500000/csi-bridge@4504000: failed to match any schema with compatible: ['ti,j721e-csi2rx', 'cdns,csi2rx']
> Documentation/devicetree/bindings/media/ti,j721e-csi2rx-shim.example.dtb: /example-0/ticsi2rx@4500000/csi-bridge@4504000: failed to match any schema with compatible: ['ti,j721e-csi2rx', 'cdns,csi2rx']

As noted, this can be ignored.

Rob