| Message ID | 20201023174546.504028-1-paul.kocialkowski@bootlin.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Allwinner MIPI CSI-2 support for A31/V3s/A83T | expand |
On Fri, Oct 23, 2020 at 07:45:32PM +0200, Paul Kocialkowski wrote: > This series introduces support for MIPI CSI-2, with the A31 controller that is > found on most SoCs (A31, V3s and probably V5) as well as the A83T-specific > controller. While the former uses the same MIPI D-PHY that is already supported > for DSI, the latter embeds its own D-PHY. > > In order to distinguish the use of the D-PHY between Rx mode (for MIPI CSI-2) > and Tx mode (for MIPI DSI), a submode is introduced for D-PHY in the PHY API. > This allows adding Rx support in the A31 D-PHY driver. > > A few changes and fixes are applied to the A31 CSI controller driver, in order > to support the MIPI CSI-2 use-case. > > Follows is the V4L2 device topology representing the interactions between > the MIPI CSI-2 sensor, the MIPI CSI-2 controller (which controls the D-PHY) > and the CSI controller: > - entity 1: sun6i-csi (1 pad, 1 link) > type Node subtype V4L flags 0 > device node name /dev/video0 > pad0: Sink > <- "sun6i-mipi-csi2":1 [ENABLED,IMMUTABLE] > > - entity 5: sun6i-mipi-csi2 (2 pads, 2 links) > type V4L2 subdev subtype Unknown flags 0 > pad0: Sink > <- "ov5648 0-0036":0 [ENABLED,IMMUTABLE] > pad1: Source > -> "sun6i-csi":0 [ENABLED,IMMUTABLE] > > - entity 8: ov5648 0-0036 (1 pad, 1 link) > type V4L2 subdev subtype Sensor flags 0 > device node name /dev/v4l-subdev0 > pad0: Source > [fmt:SBGGR8_1X8/640x480@1/30 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range] > -> "sun6i-mipi-csi2":0 [ENABLED,IMMUTABLE] > > Happy reviewing! I mentioned it to Kevin in the first version, but you should have a v4l2-compliance run here. If you have some time, it would be great to run libcamera as well. Maxime
Hi Paul, I have some comments through the series, I hope this helps. On 10/23/20 2:45 PM, Paul Kocialkowski wrote: > This series introduces support for MIPI CSI-2, with the A31 controller that is > found on most SoCs (A31, V3s and probably V5) as well as the A83T-specific > controller. While the former uses the same MIPI D-PHY that is already supported > for DSI, the latter embeds its own D-PHY. > > In order to distinguish the use of the D-PHY between Rx mode (for MIPI CSI-2) > and Tx mode (for MIPI DSI), a submode is introduced for D-PHY in the PHY API. > This allows adding Rx support in the A31 D-PHY driver. > > A few changes and fixes are applied to the A31 CSI controller driver, in order > to support the MIPI CSI-2 use-case. > > Follows is the V4L2 device topology representing the interactions between > the MIPI CSI-2 sensor, the MIPI CSI-2 controller (which controls the D-PHY) > and the CSI controller: > - entity 1: sun6i-csi (1 pad, 1 link) > type Node subtype V4L flags 0 > device node name /dev/video0 > pad0: Sink > <- "sun6i-mipi-csi2":1 [ENABLED,IMMUTABLE] > > - entity 5: sun6i-mipi-csi2 (2 pads, 2 links) > type V4L2 subdev subtype Unknown flags 0 > pad0: Sink > <- "ov5648 0-0036":0 [ENABLED,IMMUTABLE] > pad1: Source > -> "sun6i-csi":0 [ENABLED,IMMUTABLE] > > - entity 8: ov5648 0-0036 (1 pad, 1 link) > type V4L2 subdev subtype Sensor flags 0 > device node name /dev/v4l-subdev0 Question: I noticed is that sun6i-mipi-csi2 doesn't expose a node under /dev/, but the sensor exposes it. Probably because it uses V4L2_SUBDEV_FL_HAS_DEVNODE and sun6i-csi() calls v4l2_device_register_subdev_nodes(). I find this weird from a userspace pov, since usually we don't mix manual and auto propagation of the configs, so I started wondering if sun6i-csi driver should be calling v4l2_device_register_subdev_nodes() in the first place. Also, sun6i-csi doesn't seem to be used by any board dts (it's declared on the dtsi, but I didn't find any dts enabling it), so I wonder if it would be a bad thing if we update it. > pad0: Source > [fmt:SBGGR8_1X8/640x480@1/30 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range] > -> "sun6i-mipi-csi2":0 [ENABLED,IMMUTABLE] If I understand correctly, this is very similar to ipu3: sensor->bus->dma_engine in the case of ipu3-cio2: sensor->ipu3-csi2->ipu3-cio2 in this case: ov5648->sun6i-mipi-csi2->sun6i-csi On thing that is confusing me is the name csi2 with csi (that makes me think of csi vesun6i-csirsion one, which is not the case), I would rename it to sun6i-video (or maybe it is just me who gets confused). I know this driver is already upstream and not part of this series, but on the other hand it doesn't seem to be used. On another note, I always wonder if we should expose the bus in the topology, I'm not sure if it provides any useful API or information for userspace, and you could have a cleaner code (maybe code could be under phy subsystem). But at the same time, it seems this is a pattern on v4l2. I'd like to hear what others think on the above. Regards, Helen > > Happy reviewing! > > Paul Kocialkowski (14): > phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes > phy: allwinner: phy-sun6i-mipi-dphy: Support D-PHY Rx mode for MIPI > CSI-2 > media: sun6i-csi: Support an optional dedicated memory pool > media: sun6i-csi: Fix the image storage bpp for 10/12-bit Bayer > formats > media: sun6i-csi: Only configure the interface data width for parallel > media: sun6i-csi: Support feeding from the MIPI CSI-2 controller > dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation > media: sunxi: Add support for the A31 MIPI CSI-2 controller > ARM: dts: sun8i: v3s: Add CSI0 camera interface node > ARM: dts: sun8i: v3s: Add MIPI D-PHY and MIPI CSI-2 interface nodes > dt-bindings: media: i2c: Add A83T MIPI CSI-2 bindings documentation > media: sunxi: Add support for the A83T MIPI CSI-2 controller > ARM: dts: sun8i: a83t: Add MIPI CSI-2 controller node > media: sunxi: sun8i-a83t-mipi-csi2: Avoid using the (unsolicited) > interrupt > > .../media/allwinner,sun6i-a31-mipi-csi2.yaml | 168 +++++ > .../media/allwinner,sun8i-a83t-mipi-csi2.yaml | 158 +++++ > arch/arm/boot/dts/sun8i-a83t.dtsi | 26 + > arch/arm/boot/dts/sun8i-v3s.dtsi | 62 ++ > drivers/media/platform/sunxi/Kconfig | 2 + > drivers/media/platform/sunxi/Makefile | 2 + > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 54 +- > .../platform/sunxi/sun6i-csi/sun6i_csi.h | 20 +- > .../platform/sunxi/sun6i-mipi-csi2/Kconfig | 11 + > .../platform/sunxi/sun6i-mipi-csi2/Makefile | 4 + > .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 635 +++++++++++++++++ > .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h | 116 +++ > .../sunxi/sun8i-a83t-mipi-csi2/Kconfig | 11 + > .../sunxi/sun8i-a83t-mipi-csi2/Makefile | 4 + > .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c | 92 +++ > .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h | 39 ++ > .../sun8i_a83t_mipi_csi2.c | 660 ++++++++++++++++++ > .../sun8i_a83t_mipi_csi2.h | 196 ++++++ > drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 164 ++++- > drivers/staging/media/rkisp1/rkisp1-isp.c | 3 +- > include/linux/phy/phy-mipi-dphy.h | 13 + > 21 files changed, 2408 insertions(+), 32 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml > create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun8i-a83t-mipi-csi2.yaml > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Kconfig > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Makefile > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Kconfig > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Makefile > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h >
Hi On Fri, Oct 30, 2020 at 07:44:28PM -0300, Helen Koike wrote: > On thing that is confusing me is the name csi2 with csi (that makes me > think of csi vesun6i-csirsion one, which is not the case), I would > rename it to sun6i-video (or maybe it is just me who gets confused). > > I know this driver is already upstream and not part of this series, > but on the other hand it doesn't seem to be used. It's definitely confusing but CSI is the name of the IP, but it supports more than just MIPI-CSI :) Maxime
Hi Helen, On Fri 30 Oct 20, 19:44, Helen Koike wrote: > Hi Paul, > > I have some comments through the series, I hope this helps. Thanks for your comments :) > On 10/23/20 2:45 PM, Paul Kocialkowski wrote: > > This series introduces support for MIPI CSI-2, with the A31 controller that is > > found on most SoCs (A31, V3s and probably V5) as well as the A83T-specific > > controller. While the former uses the same MIPI D-PHY that is already supported > > for DSI, the latter embeds its own D-PHY. > > > > In order to distinguish the use of the D-PHY between Rx mode (for MIPI CSI-2) > > and Tx mode (for MIPI DSI), a submode is introduced for D-PHY in the PHY API. > > This allows adding Rx support in the A31 D-PHY driver. > > > > A few changes and fixes are applied to the A31 CSI controller driver, in order > > to support the MIPI CSI-2 use-case. > > > > Follows is the V4L2 device topology representing the interactions between > > the MIPI CSI-2 sensor, the MIPI CSI-2 controller (which controls the D-PHY) > > and the CSI controller: > > - entity 1: sun6i-csi (1 pad, 1 link) > > type Node subtype V4L flags 0 > > device node name /dev/video0 > > pad0: Sink > > <- "sun6i-mipi-csi2":1 [ENABLED,IMMUTABLE] > > > > - entity 5: sun6i-mipi-csi2 (2 pads, 2 links) > > type V4L2 subdev subtype Unknown flags 0 > > pad0: Sink > > <- "ov5648 0-0036":0 [ENABLED,IMMUTABLE] > > pad1: Source > > -> "sun6i-csi":0 [ENABLED,IMMUTABLE] > > > > - entity 8: ov5648 0-0036 (1 pad, 1 link) > > type V4L2 subdev subtype Sensor flags 0 > > device node name /dev/v4l-subdev0 > > Question: I noticed is that sun6i-mipi-csi2 doesn't expose a node under /dev/, but the sensor > exposes it. Probably because it uses V4L2_SUBDEV_FL_HAS_DEVNODE and sun6i-csi() calls > v4l2_device_register_subdev_nodes(). > > I find this weird from a userspace pov, since usually we don't mix manual and auto propagation > of the configs, so I started wondering if sun6i-csi driver should be calling > v4l2_device_register_subdev_nodes() in the first place. I must admit that I didn't really pay attention to that, but since sun6i-mipi-csi2 is basically a bridge driver, it doesn't make sense to apply manual configuration to it. It is actually designed to forward most subdev ops to its own subdev so configuring it manually would actually result in configuring the sensor. XXX > Also, sun6i-csi doesn't seem to be used by any board dts (it's declared on the dtsi, but I > didn't find any dts enabling it), so I wonder if it would be a bad thing if we update it. > > > pad0: Source > > [fmt:SBGGR8_1X8/640x480@1/30 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range] > > -> "sun6i-mipi-csi2":0 [ENABLED,IMMUTABLE] > > If I understand correctly, this is very similar to ipu3: > sensor->bus->dma_engine > > in the case of ipu3-cio2: > sensor->ipu3-csi2->ipu3-cio2 > > in this case: > ov5648->sun6i-mipi-csi2->sun6i-csi Yes this is the correct picture. > On thing that is confusing me is the name csi2 with csi (that makes me think of csi > version one, which is not the case), I would rename it to sun6i-video (or maybe > it is just me who gets confused). So the CSI name comes from the Allwinner litterature and implementation for that controller. Since it supports parallel input on its own, it does in fact support parallel CSI. The DMA engine part alone from that controller is also used for MIPI CSI-2, so in this case the name looses its relevance. > I know this driver is already upstream and not part of this series, but on the other hand it > doesn't seem to be used. Personally I don't find a rename to be necessary and while I agree that nothing would apparently prevent us from renaming it, I would prefer to keep the naming in line with Allwinner's litterature. > On another note, I always wonder if we should expose the bus in the topology, I'm not > sure if it provides any useful API or information for userspace, and you could have > a cleaner code (maybe code could be under phy subsystem). But at the same time, it > seems this is a pattern on v4l2. > > I'd like to hear what others think on the above. My view on this is that we are dealing with two distinct controllers here, one that acts as a DMA engine and one that acts as a bridge. As a result, two chained subdevs looks like the most appropriate representation to me. Using the PHY subsystem would probably be abusing the framework since the MIPI CSI-2 controller is not a PHY (and we do have a D-PHY driver for the D-PHY part that uses the PHY API already). So tl;dr I don't agree that it would be cleaner. Cheers, Paul > > Happy reviewing! > > > > Paul Kocialkowski (14): > > phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes > > phy: allwinner: phy-sun6i-mipi-dphy: Support D-PHY Rx mode for MIPI > > CSI-2 > > media: sun6i-csi: Support an optional dedicated memory pool > > media: sun6i-csi: Fix the image storage bpp for 10/12-bit Bayer > > formats > > media: sun6i-csi: Only configure the interface data width for parallel > > media: sun6i-csi: Support feeding from the MIPI CSI-2 controller > > dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation > > media: sunxi: Add support for the A31 MIPI CSI-2 controller > > ARM: dts: sun8i: v3s: Add CSI0 camera interface node > > ARM: dts: sun8i: v3s: Add MIPI D-PHY and MIPI CSI-2 interface nodes > > dt-bindings: media: i2c: Add A83T MIPI CSI-2 bindings documentation > > media: sunxi: Add support for the A83T MIPI CSI-2 controller > > ARM: dts: sun8i: a83t: Add MIPI CSI-2 controller node > > media: sunxi: sun8i-a83t-mipi-csi2: Avoid using the (unsolicited) > > interrupt > > > > .../media/allwinner,sun6i-a31-mipi-csi2.yaml | 168 +++++ > > .../media/allwinner,sun8i-a83t-mipi-csi2.yaml | 158 +++++ > > arch/arm/boot/dts/sun8i-a83t.dtsi | 26 + > > arch/arm/boot/dts/sun8i-v3s.dtsi | 62 ++ > > drivers/media/platform/sunxi/Kconfig | 2 + > > drivers/media/platform/sunxi/Makefile | 2 + > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 54 +- > > .../platform/sunxi/sun6i-csi/sun6i_csi.h | 20 +- > > .../platform/sunxi/sun6i-mipi-csi2/Kconfig | 11 + > > .../platform/sunxi/sun6i-mipi-csi2/Makefile | 4 + > > .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 635 +++++++++++++++++ > > .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h | 116 +++ > > .../sunxi/sun8i-a83t-mipi-csi2/Kconfig | 11 + > > .../sunxi/sun8i-a83t-mipi-csi2/Makefile | 4 + > > .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c | 92 +++ > > .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h | 39 ++ > > .../sun8i_a83t_mipi_csi2.c | 660 ++++++++++++++++++ > > .../sun8i_a83t_mipi_csi2.h | 196 ++++++ > > drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 164 ++++- > > drivers/staging/media/rkisp1/rkisp1-isp.c | 3 +- > > include/linux/phy/phy-mipi-dphy.h | 13 + > > 21 files changed, 2408 insertions(+), 32 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml > > create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun8i-a83t-mipi-csi2.yaml > > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Kconfig > > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Makefile > > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c > > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Kconfig > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Makefile > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h > >
Hi again, On Wed 04 Nov 20, 12:11, Paul Kocialkowski wrote: > Hi Helen, > > On Fri 30 Oct 20, 19:44, Helen Koike wrote: > > Hi Paul, > > > > I have some comments through the series, I hope this helps. > > Thanks for your comments :) > > > On 10/23/20 2:45 PM, Paul Kocialkowski wrote: > > > This series introduces support for MIPI CSI-2, with the A31 controller that is > > > found on most SoCs (A31, V3s and probably V5) as well as the A83T-specific > > > controller. While the former uses the same MIPI D-PHY that is already supported > > > for DSI, the latter embeds its own D-PHY. > > > > > > In order to distinguish the use of the D-PHY between Rx mode (for MIPI CSI-2) > > > and Tx mode (for MIPI DSI), a submode is introduced for D-PHY in the PHY API. > > > This allows adding Rx support in the A31 D-PHY driver. > > > > > > A few changes and fixes are applied to the A31 CSI controller driver, in order > > > to support the MIPI CSI-2 use-case. > > > > > > Follows is the V4L2 device topology representing the interactions between > > > the MIPI CSI-2 sensor, the MIPI CSI-2 controller (which controls the D-PHY) > > > and the CSI controller: > > > - entity 1: sun6i-csi (1 pad, 1 link) > > > type Node subtype V4L flags 0 > > > device node name /dev/video0 > > > pad0: Sink > > > <- "sun6i-mipi-csi2":1 [ENABLED,IMMUTABLE] > > > > > > - entity 5: sun6i-mipi-csi2 (2 pads, 2 links) > > > type V4L2 subdev subtype Unknown flags 0 > > > pad0: Sink > > > <- "ov5648 0-0036":0 [ENABLED,IMMUTABLE] > > > pad1: Source > > > -> "sun6i-csi":0 [ENABLED,IMMUTABLE] > > > > > > - entity 8: ov5648 0-0036 (1 pad, 1 link) > > > type V4L2 subdev subtype Sensor flags 0 > > > device node name /dev/v4l-subdev0 > > > > Question: I noticed is that sun6i-mipi-csi2 doesn't expose a node under /dev/, but the sensor > > exposes it. Probably because it uses V4L2_SUBDEV_FL_HAS_DEVNODE and sun6i-csi() calls > > v4l2_device_register_subdev_nodes(). > > > > I find this weird from a userspace pov, since usually we don't mix manual and auto propagation > > of the configs, so I started wondering if sun6i-csi driver should be calling > > v4l2_device_register_subdev_nodes() in the first place. > > I must admit that I didn't really pay attention to that, but since > sun6i-mipi-csi2 is basically a bridge driver, it doesn't make sense to apply > manual configuration to it. It is actually designed to forward most subdev ops > to its own subdev so configuring it manually would actually result in > configuring the sensor. > > XXX Hum, I meant to add something here and then forgot. I'm pretty new to auto vs manual propagation so I don't really have a clear opinion on this and whether we should consider removing the sensor /dev node as well. I'm satisfied with the way things are currently, but it might be due to my own ignorance. Paul > > Also, sun6i-csi doesn't seem to be used by any board dts (it's declared on the dtsi, but I > > didn't find any dts enabling it), so I wonder if it would be a bad thing if we update it. > > > > > pad0: Source > > > [fmt:SBGGR8_1X8/640x480@1/30 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range] > > > -> "sun6i-mipi-csi2":0 [ENABLED,IMMUTABLE] > > > > If I understand correctly, this is very similar to ipu3: > > sensor->bus->dma_engine > > > > in the case of ipu3-cio2: > > sensor->ipu3-csi2->ipu3-cio2 > > > > in this case: > > ov5648->sun6i-mipi-csi2->sun6i-csi > > Yes this is the correct picture. > > > On thing that is confusing me is the name csi2 with csi (that makes me think of csi > > version one, which is not the case), I would rename it to sun6i-video (or maybe > > it is just me who gets confused). > > So the CSI name comes from the Allwinner litterature and implementation for that > controller. Since it supports parallel input on its own, it does in fact support > parallel CSI. The DMA engine part alone from that controller is also used for > MIPI CSI-2, so in this case the name looses its relevance. > > > I know this driver is already upstream and not part of this series, but on the other hand it > > doesn't seem to be used. > > Personally I don't find a rename to be necessary and while I agree that > nothing would apparently prevent us from renaming it, I would prefer to keep > the naming in line with Allwinner's litterature. > > > On another note, I always wonder if we should expose the bus in the topology, I'm not > > sure if it provides any useful API or information for userspace, and you could have > > a cleaner code (maybe code could be under phy subsystem). But at the same time, it > > seems this is a pattern on v4l2. > > > > I'd like to hear what others think on the above. > > My view on this is that we are dealing with two distinct controllers here, > one that acts as a DMA engine and one that acts as a bridge. As a result, two > chained subdevs looks like the most appropriate representation to me. > > Using the PHY subsystem would probably be abusing the framework since the > MIPI CSI-2 controller is not a PHY (and we do have a D-PHY driver for the D-PHY > part that uses the PHY API already). > > So tl;dr I don't agree that it would be cleaner. > > Cheers, > > Paul > > > > Happy reviewing! > > > > > > Paul Kocialkowski (14): > > > phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes > > > phy: allwinner: phy-sun6i-mipi-dphy: Support D-PHY Rx mode for MIPI > > > CSI-2 > > > media: sun6i-csi: Support an optional dedicated memory pool > > > media: sun6i-csi: Fix the image storage bpp for 10/12-bit Bayer > > > formats > > > media: sun6i-csi: Only configure the interface data width for parallel > > > media: sun6i-csi: Support feeding from the MIPI CSI-2 controller > > > dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation > > > media: sunxi: Add support for the A31 MIPI CSI-2 controller > > > ARM: dts: sun8i: v3s: Add CSI0 camera interface node > > > ARM: dts: sun8i: v3s: Add MIPI D-PHY and MIPI CSI-2 interface nodes > > > dt-bindings: media: i2c: Add A83T MIPI CSI-2 bindings documentation > > > media: sunxi: Add support for the A83T MIPI CSI-2 controller > > > ARM: dts: sun8i: a83t: Add MIPI CSI-2 controller node > > > media: sunxi: sun8i-a83t-mipi-csi2: Avoid using the (unsolicited) > > > interrupt > > > > > > .../media/allwinner,sun6i-a31-mipi-csi2.yaml | 168 +++++ > > > .../media/allwinner,sun8i-a83t-mipi-csi2.yaml | 158 +++++ > > > arch/arm/boot/dts/sun8i-a83t.dtsi | 26 + > > > arch/arm/boot/dts/sun8i-v3s.dtsi | 62 ++ > > > drivers/media/platform/sunxi/Kconfig | 2 + > > > drivers/media/platform/sunxi/Makefile | 2 + > > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 54 +- > > > .../platform/sunxi/sun6i-csi/sun6i_csi.h | 20 +- > > > .../platform/sunxi/sun6i-mipi-csi2/Kconfig | 11 + > > > .../platform/sunxi/sun6i-mipi-csi2/Makefile | 4 + > > > .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 635 +++++++++++++++++ > > > .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h | 116 +++ > > > .../sunxi/sun8i-a83t-mipi-csi2/Kconfig | 11 + > > > .../sunxi/sun8i-a83t-mipi-csi2/Makefile | 4 + > > > .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c | 92 +++ > > > .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h | 39 ++ > > > .../sun8i_a83t_mipi_csi2.c | 660 ++++++++++++++++++ > > > .../sun8i_a83t_mipi_csi2.h | 196 ++++++ > > > drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 164 ++++- > > > drivers/staging/media/rkisp1/rkisp1-isp.c | 3 +- > > > include/linux/phy/phy-mipi-dphy.h | 13 + > > > 21 files changed, 2408 insertions(+), 32 deletions(-) > > > create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml > > > create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun8i-a83t-mipi-csi2.yaml > > > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Kconfig > > > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Makefile > > > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c > > > create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h > > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Kconfig > > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Makefile > > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c > > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h > > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c > > > create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h > > > > > -- > Paul Kocialkowski, Bootlin > Embedded Linux and kernel engineering > https://bootlin.com
Hi Paul, On 11/4/20 8:11 AM, Paul Kocialkowski wrote: > Hi Helen, > > On Fri 30 Oct 20, 19:44, Helen Koike wrote: >> Hi Paul, >> >> I have some comments through the series, I hope this helps. > > Thanks for your comments :) > >> On 10/23/20 2:45 PM, Paul Kocialkowski wrote: >>> This series introduces support for MIPI CSI-2, with the A31 controller that is >>> found on most SoCs (A31, V3s and probably V5) as well as the A83T-specific >>> controller. While the former uses the same MIPI D-PHY that is already supported >>> for DSI, the latter embeds its own D-PHY. >>> >>> In order to distinguish the use of the D-PHY between Rx mode (for MIPI CSI-2) >>> and Tx mode (for MIPI DSI), a submode is introduced for D-PHY in the PHY API. >>> This allows adding Rx support in the A31 D-PHY driver. >>> >>> A few changes and fixes are applied to the A31 CSI controller driver, in order >>> to support the MIPI CSI-2 use-case. >>> >>> Follows is the V4L2 device topology representing the interactions between >>> the MIPI CSI-2 sensor, the MIPI CSI-2 controller (which controls the D-PHY) >>> and the CSI controller: >>> - entity 1: sun6i-csi (1 pad, 1 link) >>> type Node subtype V4L flags 0 >>> device node name /dev/video0 >>> pad0: Sink >>> <- "sun6i-mipi-csi2":1 [ENABLED,IMMUTABLE] >>> >>> - entity 5: sun6i-mipi-csi2 (2 pads, 2 links) >>> type V4L2 subdev subtype Unknown flags 0 >>> pad0: Sink >>> <- "ov5648 0-0036":0 [ENABLED,IMMUTABLE] >>> pad1: Source >>> -> "sun6i-csi":0 [ENABLED,IMMUTABLE] >>> >>> - entity 8: ov5648 0-0036 (1 pad, 1 link) >>> type V4L2 subdev subtype Sensor flags 0 >>> device node name /dev/v4l-subdev0 >> >> Question: I noticed is that sun6i-mipi-csi2 doesn't expose a node under /dev/, but the sensor >> exposes it. Probably because it uses V4L2_SUBDEV_FL_HAS_DEVNODE and sun6i-csi() calls >> v4l2_device_register_subdev_nodes(). >> >> I find this weird from a userspace pov, since usually we don't mix manual and auto propagation >> of the configs, so I started wondering if sun6i-csi driver should be calling >> v4l2_device_register_subdev_nodes() in the first place. > > I must admit that I didn't really pay attention to that, but since > sun6i-mipi-csi2 is basically a bridge driver, it doesn't make sense to apply > manual configuration to it. It is actually designed to forward most subdev ops > to its own subdev so configuring it manually would actually result in > configuring the sensor. Ack, then maybe sun6i-csi needs a patch removing the call to v4l2_device_register_subdev_nodes() > > XXX > >> Also, sun6i-csi doesn't seem to be used by any board dts (it's declared on the dtsi, but I >> didn't find any dts enabling it), so I wonder if it would be a bad thing if we update it. >> >>> pad0: Source >>> [fmt:SBGGR8_1X8/640x480@1/30 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range] >>> -> "sun6i-mipi-csi2":0 [ENABLED,IMMUTABLE] >> >> If I understand correctly, this is very similar to ipu3: >> sensor->bus->dma_engine >> >> in the case of ipu3-cio2: >> sensor->ipu3-csi2->ipu3-cio2 >> >> in this case: >> ov5648->sun6i-mipi-csi2->sun6i-csi > > Yes this is the correct picture. > >> On thing that is confusing me is the name csi2 with csi (that makes me think of csi >> version one, which is not the case), I would rename it to sun6i-video (or maybe >> it is just me who gets confused). > > So the CSI name comes from the Allwinner litterature and implementation for that > controller. Since it supports parallel input on its own, it does in fact support > parallel CSI. The DMA engine part alone from that controller is also used for > MIPI CSI-2, so in this case the name looses its relevance. > >> I know this driver is already upstream and not part of this series, but on the other hand it >> doesn't seem to be used. > > Personally I don't find a rename to be necessary and while I agree that > nothing would apparently prevent us from renaming it, I would prefer to keep > the naming in line with Allwinner's litterature. Ok, I didn't know it was from Allwinner's litterature, I don't mind keeping the name. > >> On another note, I always wonder if we should expose the bus in the topology, I'm not >> sure if it provides any useful API or information for userspace, and you could have >> a cleaner code (maybe code could be under phy subsystem). But at the same time, it >> seems this is a pattern on v4l2. >> >> I'd like to hear what others think on the above. > > My view on this is that we are dealing with two distinct controllers here, > one that acts as a DMA engine and one that acts as a bridge. As a result, two > chained subdevs looks like the most appropriate representation to me. > > Using the PHY subsystem would probably be abusing the framework since the > MIPI CSI-2 controller is not a PHY (and we do have a D-PHY driver for the D-PHY > part that uses the PHY API already). > > So tl;dr I don't agree that it would be cleaner. My point is, this is a "dummy" subdevice in userspace pov, but if it is only used with auto-propagation of the configurations, then it doesn't matter (since userspace won't interact with that node). And in the kernel space you need to implement media boilerplate code. So I was trying to think in another alternative, but tbh I don't mind keeping it in the media topology. Regards, Helen > > Cheers, > > Paul > >>> Happy reviewing! >>> >>> Paul Kocialkowski (14): >>> phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes >>> phy: allwinner: phy-sun6i-mipi-dphy: Support D-PHY Rx mode for MIPI >>> CSI-2 >>> media: sun6i-csi: Support an optional dedicated memory pool >>> media: sun6i-csi: Fix the image storage bpp for 10/12-bit Bayer >>> formats >>> media: sun6i-csi: Only configure the interface data width for parallel >>> media: sun6i-csi: Support feeding from the MIPI CSI-2 controller >>> dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation >>> media: sunxi: Add support for the A31 MIPI CSI-2 controller >>> ARM: dts: sun8i: v3s: Add CSI0 camera interface node >>> ARM: dts: sun8i: v3s: Add MIPI D-PHY and MIPI CSI-2 interface nodes >>> dt-bindings: media: i2c: Add A83T MIPI CSI-2 bindings documentation >>> media: sunxi: Add support for the A83T MIPI CSI-2 controller >>> ARM: dts: sun8i: a83t: Add MIPI CSI-2 controller node >>> media: sunxi: sun8i-a83t-mipi-csi2: Avoid using the (unsolicited) >>> interrupt >>> >>> .../media/allwinner,sun6i-a31-mipi-csi2.yaml | 168 +++++ >>> .../media/allwinner,sun8i-a83t-mipi-csi2.yaml | 158 +++++ >>> arch/arm/boot/dts/sun8i-a83t.dtsi | 26 + >>> arch/arm/boot/dts/sun8i-v3s.dtsi | 62 ++ >>> drivers/media/platform/sunxi/Kconfig | 2 + >>> drivers/media/platform/sunxi/Makefile | 2 + >>> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 54 +- >>> .../platform/sunxi/sun6i-csi/sun6i_csi.h | 20 +- >>> .../platform/sunxi/sun6i-mipi-csi2/Kconfig | 11 + >>> .../platform/sunxi/sun6i-mipi-csi2/Makefile | 4 + >>> .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 635 +++++++++++++++++ >>> .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h | 116 +++ >>> .../sunxi/sun8i-a83t-mipi-csi2/Kconfig | 11 + >>> .../sunxi/sun8i-a83t-mipi-csi2/Makefile | 4 + >>> .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c | 92 +++ >>> .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h | 39 ++ >>> .../sun8i_a83t_mipi_csi2.c | 660 ++++++++++++++++++ >>> .../sun8i_a83t_mipi_csi2.h | 196 ++++++ >>> drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 164 ++++- >>> drivers/staging/media/rkisp1/rkisp1-isp.c | 3 +- >>> include/linux/phy/phy-mipi-dphy.h | 13 + >>> 21 files changed, 2408 insertions(+), 32 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml >>> create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun8i-a83t-mipi-csi2.yaml >>> create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Kconfig >>> create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Makefile >>> create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c >>> create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Kconfig >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Makefile >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h >>> >
Hi, On Wed 04 Nov 20, 13:36, Helen Koike wrote: > Hi Paul, > > On 11/4/20 8:11 AM, Paul Kocialkowski wrote: > > Hi Helen, > > > > On Fri 30 Oct 20, 19:44, Helen Koike wrote: > >> Hi Paul, > >> > >> I have some comments through the series, I hope this helps. > > > > Thanks for your comments :) > > > >> On 10/23/20 2:45 PM, Paul Kocialkowski wrote: > >>> This series introduces support for MIPI CSI-2, with the A31 controller that is > >>> found on most SoCs (A31, V3s and probably V5) as well as the A83T-specific > >>> controller. While the former uses the same MIPI D-PHY that is already supported > >>> for DSI, the latter embeds its own D-PHY. > >>> > >>> In order to distinguish the use of the D-PHY between Rx mode (for MIPI CSI-2) > >>> and Tx mode (for MIPI DSI), a submode is introduced for D-PHY in the PHY API. > >>> This allows adding Rx support in the A31 D-PHY driver. > >>> > >>> A few changes and fixes are applied to the A31 CSI controller driver, in order > >>> to support the MIPI CSI-2 use-case. > >>> > >>> Follows is the V4L2 device topology representing the interactions between > >>> the MIPI CSI-2 sensor, the MIPI CSI-2 controller (which controls the D-PHY) > >>> and the CSI controller: > >>> - entity 1: sun6i-csi (1 pad, 1 link) > >>> type Node subtype V4L flags 0 > >>> device node name /dev/video0 > >>> pad0: Sink > >>> <- "sun6i-mipi-csi2":1 [ENABLED,IMMUTABLE] > >>> > >>> - entity 5: sun6i-mipi-csi2 (2 pads, 2 links) > >>> type V4L2 subdev subtype Unknown flags 0 > >>> pad0: Sink > >>> <- "ov5648 0-0036":0 [ENABLED,IMMUTABLE] > >>> pad1: Source > >>> -> "sun6i-csi":0 [ENABLED,IMMUTABLE] > >>> > >>> - entity 8: ov5648 0-0036 (1 pad, 1 link) > >>> type V4L2 subdev subtype Sensor flags 0 > >>> device node name /dev/v4l-subdev0 > >> > >> Question: I noticed is that sun6i-mipi-csi2 doesn't expose a node under /dev/, but the sensor > >> exposes it. Probably because it uses V4L2_SUBDEV_FL_HAS_DEVNODE and sun6i-csi() calls > >> v4l2_device_register_subdev_nodes(). > >> > >> I find this weird from a userspace pov, since usually we don't mix manual and auto propagation > >> of the configs, so I started wondering if sun6i-csi driver should be calling > >> v4l2_device_register_subdev_nodes() in the first place. > > > > I must admit that I didn't really pay attention to that, but since > > sun6i-mipi-csi2 is basically a bridge driver, it doesn't make sense to apply > > manual configuration to it. It is actually designed to forward most subdev ops > > to its own subdev so configuring it manually would actually result in > > configuring the sensor. > > Ack, then maybe sun6i-csi needs a patch removing the call to v4l2_device_register_subdev_nodes() Apparently Sakari suggested that we do need a subdev node for the MIPI CSI-2 bridge so I'll just do that. This implementation that fowards the ops to the sensor was apparently a mistake. Paul > > > > XXX > > > >> Also, sun6i-csi doesn't seem to be used by any board dts (it's declared on the dtsi, but I > >> didn't find any dts enabling it), so I wonder if it would be a bad thing if we update it. > >> > >>> pad0: Source > >>> [fmt:SBGGR8_1X8/640x480@1/30 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range] > >>> -> "sun6i-mipi-csi2":0 [ENABLED,IMMUTABLE] > >> > >> If I understand correctly, this is very similar to ipu3: > >> sensor->bus->dma_engine > >> > >> in the case of ipu3-cio2: > >> sensor->ipu3-csi2->ipu3-cio2 > >> > >> in this case: > >> ov5648->sun6i-mipi-csi2->sun6i-csi > > > > Yes this is the correct picture. > > > >> On thing that is confusing me is the name csi2 with csi (that makes me think of csi > >> version one, which is not the case), I would rename it to sun6i-video (or maybe > >> it is just me who gets confused). > > > > So the CSI name comes from the Allwinner litterature and implementation for that > > controller. Since it supports parallel input on its own, it does in fact support > > parallel CSI. The DMA engine part alone from that controller is also used for > > MIPI CSI-2, so in this case the name looses its relevance. > > > >> I know this driver is already upstream and not part of this series, but on the other hand it > >> doesn't seem to be used. > > > > Personally I don't find a rename to be necessary and while I agree that > > nothing would apparently prevent us from renaming it, I would prefer to keep > > the naming in line with Allwinner's litterature. > > Ok, I didn't know it was from Allwinner's litterature, I don't mind keeping the name. Great :) > >> On another note, I always wonder if we should expose the bus in the topology, I'm not > >> sure if it provides any useful API or information for userspace, and you could have > >> a cleaner code (maybe code could be under phy subsystem). But at the same time, it > >> seems this is a pattern on v4l2. > >> > >> I'd like to hear what others think on the above. > > > > My view on this is that we are dealing with two distinct controllers here, > > one that acts as a DMA engine and one that acts as a bridge. As a result, two > > chained subdevs looks like the most appropriate representation to me. > > > > Using the PHY subsystem would probably be abusing the framework since the > > MIPI CSI-2 controller is not a PHY (and we do have a D-PHY driver for the D-PHY > > part that uses the PHY API already). > > > > So tl;dr I don't agree that it would be cleaner. > > My point is, this is a "dummy" subdevice in userspace pov, > but if it is only used with auto-propagation of the configurations, then > it doesn't matter (since userspace won't interact with that node). > And in the kernel space you need to implement media boilerplate code. > So I was trying to think in another alternative, but tbh I don't mind > keeping it in the media topology. Undestood and thanks for sharing your thoughts either way, they are definitely welcome! Cheers, Paul > Regards, > Helen > > > > > Cheers, > > > > Paul > > > >>> Happy reviewing! > >>> > >>> Paul Kocialkowski (14): > >>> phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes > >>> phy: allwinner: phy-sun6i-mipi-dphy: Support D-PHY Rx mode for MIPI > >>> CSI-2 > >>> media: sun6i-csi: Support an optional dedicated memory pool > >>> media: sun6i-csi: Fix the image storage bpp for 10/12-bit Bayer > >>> formats > >>> media: sun6i-csi: Only configure the interface data width for parallel > >>> media: sun6i-csi: Support feeding from the MIPI CSI-2 controller > >>> dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation > >>> media: sunxi: Add support for the A31 MIPI CSI-2 controller > >>> ARM: dts: sun8i: v3s: Add CSI0 camera interface node > >>> ARM: dts: sun8i: v3s: Add MIPI D-PHY and MIPI CSI-2 interface nodes > >>> dt-bindings: media: i2c: Add A83T MIPI CSI-2 bindings documentation > >>> media: sunxi: Add support for the A83T MIPI CSI-2 controller > >>> ARM: dts: sun8i: a83t: Add MIPI CSI-2 controller node > >>> media: sunxi: sun8i-a83t-mipi-csi2: Avoid using the (unsolicited) > >>> interrupt > >>> > >>> .../media/allwinner,sun6i-a31-mipi-csi2.yaml | 168 +++++ > >>> .../media/allwinner,sun8i-a83t-mipi-csi2.yaml | 158 +++++ > >>> arch/arm/boot/dts/sun8i-a83t.dtsi | 26 + > >>> arch/arm/boot/dts/sun8i-v3s.dtsi | 62 ++ > >>> drivers/media/platform/sunxi/Kconfig | 2 + > >>> drivers/media/platform/sunxi/Makefile | 2 + > >>> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 54 +- > >>> .../platform/sunxi/sun6i-csi/sun6i_csi.h | 20 +- > >>> .../platform/sunxi/sun6i-mipi-csi2/Kconfig | 11 + > >>> .../platform/sunxi/sun6i-mipi-csi2/Makefile | 4 + > >>> .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 635 +++++++++++++++++ > >>> .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h | 116 +++ > >>> .../sunxi/sun8i-a83t-mipi-csi2/Kconfig | 11 + > >>> .../sunxi/sun8i-a83t-mipi-csi2/Makefile | 4 + > >>> .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c | 92 +++ > >>> .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h | 39 ++ > >>> .../sun8i_a83t_mipi_csi2.c | 660 ++++++++++++++++++ > >>> .../sun8i_a83t_mipi_csi2.h | 196 ++++++ > >>> drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 164 ++++- > >>> drivers/staging/media/rkisp1/rkisp1-isp.c | 3 +- > >>> include/linux/phy/phy-mipi-dphy.h | 13 + > >>> 21 files changed, 2408 insertions(+), 32 deletions(-) > >>> create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml > >>> create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun8i-a83t-mipi-csi2.yaml > >>> create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Kconfig > >>> create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Makefile > >>> create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c > >>> create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h > >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Kconfig > >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Makefile > >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c > >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h > >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c > >>> create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h > >>> > >
This series introduces support for MIPI CSI-2, with the A31 controller that is found on most SoCs (A31, V3s and probably V5) as well as the A83T-specific controller. While the former uses the same MIPI D-PHY that is already supported for DSI, the latter embeds its own D-PHY. In order to distinguish the use of the D-PHY between Rx mode (for MIPI CSI-2) and Tx mode (for MIPI DSI), a submode is introduced for D-PHY in the PHY API. This allows adding Rx support in the A31 D-PHY driver. A few changes and fixes are applied to the A31 CSI controller driver, in order to support the MIPI CSI-2 use-case. Follows is the V4L2 device topology representing the interactions between the MIPI CSI-2 sensor, the MIPI CSI-2 controller (which controls the D-PHY) and the CSI controller: - entity 1: sun6i-csi (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video0 pad0: Sink <- "sun6i-mipi-csi2":1 [ENABLED,IMMUTABLE] - entity 5: sun6i-mipi-csi2 (2 pads, 2 links) type V4L2 subdev subtype Unknown flags 0 pad0: Sink <- "ov5648 0-0036":0 [ENABLED,IMMUTABLE] pad1: Source -> "sun6i-csi":0 [ENABLED,IMMUTABLE] - entity 8: ov5648 0-0036 (1 pad, 1 link) type V4L2 subdev subtype Sensor flags 0 device node name /dev/v4l-subdev0 pad0: Source [fmt:SBGGR8_1X8/640x480@1/30 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range] -> "sun6i-mipi-csi2":0 [ENABLED,IMMUTABLE] Happy reviewing! Paul Kocialkowski (14): phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes phy: allwinner: phy-sun6i-mipi-dphy: Support D-PHY Rx mode for MIPI CSI-2 media: sun6i-csi: Support an optional dedicated memory pool media: sun6i-csi: Fix the image storage bpp for 10/12-bit Bayer formats media: sun6i-csi: Only configure the interface data width for parallel media: sun6i-csi: Support feeding from the MIPI CSI-2 controller dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation media: sunxi: Add support for the A31 MIPI CSI-2 controller ARM: dts: sun8i: v3s: Add CSI0 camera interface node ARM: dts: sun8i: v3s: Add MIPI D-PHY and MIPI CSI-2 interface nodes dt-bindings: media: i2c: Add A83T MIPI CSI-2 bindings documentation media: sunxi: Add support for the A83T MIPI CSI-2 controller ARM: dts: sun8i: a83t: Add MIPI CSI-2 controller node media: sunxi: sun8i-a83t-mipi-csi2: Avoid using the (unsolicited) interrupt .../media/allwinner,sun6i-a31-mipi-csi2.yaml | 168 +++++ .../media/allwinner,sun8i-a83t-mipi-csi2.yaml | 158 +++++ arch/arm/boot/dts/sun8i-a83t.dtsi | 26 + arch/arm/boot/dts/sun8i-v3s.dtsi | 62 ++ drivers/media/platform/sunxi/Kconfig | 2 + drivers/media/platform/sunxi/Makefile | 2 + .../platform/sunxi/sun6i-csi/sun6i_csi.c | 54 +- .../platform/sunxi/sun6i-csi/sun6i_csi.h | 20 +- .../platform/sunxi/sun6i-mipi-csi2/Kconfig | 11 + .../platform/sunxi/sun6i-mipi-csi2/Makefile | 4 + .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 635 +++++++++++++++++ .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h | 116 +++ .../sunxi/sun8i-a83t-mipi-csi2/Kconfig | 11 + .../sunxi/sun8i-a83t-mipi-csi2/Makefile | 4 + .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c | 92 +++ .../sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h | 39 ++ .../sun8i_a83t_mipi_csi2.c | 660 ++++++++++++++++++ .../sun8i_a83t_mipi_csi2.h | 196 ++++++ drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 164 ++++- drivers/staging/media/rkisp1/rkisp1-isp.c | 3 +- include/linux/phy/phy-mipi-dphy.h | 13 + 21 files changed, 2408 insertions(+), 32 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun8i-a83t-mipi-csi2.yaml create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Kconfig create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/Makefile create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c create mode 100644 drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Kconfig create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/Makefile create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.c create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_dphy.h create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c create mode 100644 drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h