[PATCH 0/6] Support second Image Signal Processor on rk3399

[PATCH 0/6] Support second Image Signal Processor on rk3399

[Heiko Stuebner]

The rk3399 has two ISPs and right now only the first one is usable.
The second ISP is connected to the TXRX dphy on the soc.

The phy of ISP1 is only accessible through the DSI controller's
io-memory, so this series adds support for simply using the dsi
controller is a phy if needed.

That solution is needed at least on rk3399 and rk3288 but no-one
has looked at camera support on rk3288 at all, so right now
only implement the rk3399 specifics.


Heiko Stuebner (6):
  drm/rockchip: dsi: add own additional pclk handling
  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
  arm64: dts: rockchip: add isp1 node on rk3399

 .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
 drivers/gpu/drm/rockchip/Kconfig              |   2 +
 .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
 4 files changed, 384 insertions(+)

-- 
2.29.2

[PATCH 1/6] drm/rockchip: dsi: add own additional pclk handling

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

In a followup patch, we'll need to access the pclk ourself to enable some
functionality, so get and store it in the rockchip dw-dsi variant as well.

Clocks are refcounted, so possible cascading enablements are
no problem.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
index 24a71091759c..18e112e30f6e 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
@@ -223,6 +223,7 @@ struct dw_mipi_dsi_rockchip {
 	void __iomem *base;
 
 	struct regmap *grf_regmap;
+	struct clk *pclk;
 	struct clk *pllref_clk;
 	struct clk *grf_clk;
 	struct clk *phy_cfg_clk;
@@ -1051,6 +1052,13 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	dsi->pclk = devm_clk_get(dev, "pclk");
+	if (IS_ERR(dsi->pclk)) {
+		ret = PTR_ERR(dsi->pclk);
+		DRM_DEV_ERROR(dev, "Unable to get pclk: %d\n", ret);
+		return ret;
+	}
+
 	dsi->pllref_clk = devm_clk_get(dev, "ref");
 	if (IS_ERR(dsi->pllref_clk)) {
 		if (dsi->phy) {
-- 
2.29.2

[PATCH 2/6] dt-bindings: display: rockchip-dsi: add optional #phy-cells property

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

The Rockchip DSI controller on some SoCs also controls a bidrectional
dphy, which would be connected to an Image Signal Processor as a phy
in the rx configuration.

So allow a #phy-cells property for the dsi controller.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 .../bindings/display/rockchip/dw_mipi_dsi_rockchip.txt           | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
index 151be3bba06f..39792f051d2d 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
@@ -23,6 +23,7 @@ Required properties:
 Optional properties:
 - phys: from general PHY binding: the phandle for the PHY device.
 - phy-names: Should be "dphy" if phys references an external phy.
+- #phy-cells: Defined when used as ISP phy, should be 0.
 - power-domains: a phandle to mipi dsi power domain node.
 - resets: list of phandle + reset specifier pairs, as described in [3].
 - reset-names: string reset name, must be "apb".
-- 
2.29.2

[PATCH 3/6] drm/rockchip: dsi: add ability to work as a phy instead of full dsi

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

SoCs like the rk3288 and rk3399 have 3 mipi dphys on them. One is TX-
only, one is RX-only and one can be configured to do either TX or RX.

The RX phy is statically connected to the first Image Signal Processor,
the TX phy is statically connected to the first DSI controller and
the TXRX phy is connected to both the second DSI controller as well
as the second ISP.

The RX dphy is controlled externally through registers in the "General
Register Files", while the other two are controlled through the
"Configuration and Test Interface" inside their DSI controller's
io-memory area.

The Rockchip dw-dsi controller already controls these dphys for the
TX case in the driver, but when we want to also allow configuration
for RX to the ISP from the media subsystem we need to expose phy-
functionality instead.

So add a bit of infrastructure to allow the dsi driver to work as a
phy and make sure it can be only one or the other at a time.

Similarly as the dsi-controller will be part of the drm-graph when
active, add an empty component to the drm-graph when in phy-mode
to make the rest of the drm-graph not wait for it.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 drivers/gpu/drm/rockchip/Kconfig              |   2 +
 .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 334 ++++++++++++++++++
 2 files changed, 336 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index cb25c0e8fc9b..3094d4533ad6 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -9,6 +9,8 @@ config DRM_ROCKCHIP
 	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
 	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
 	select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
+	select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
+	select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI
 	select DRM_RGB if ROCKCHIP_RGB
 	select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
 	help
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
index 18e112e30f6e..5988a105c141 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
@@ -14,6 +14,7 @@
 #include <linux/of_device.h>
 #include <linux/phy/phy.h>
 #include <linux/pm_runtime.h>
+#include <linux/phy/phy.h>
 #include <linux/regmap.h>
 
 #include <video/mipi_display.h>
@@ -125,7 +126,9 @@
 #define BANDGAP_AND_BIAS_CONTROL			0x20
 #define TERMINATION_RESISTER_CONTROL			0x21
 #define AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY		0x22
+#define HS_RX_CONTROL_OF_LANE_CLK			0x34
 #define HS_RX_CONTROL_OF_LANE_0				0x44
+#define HS_RX_CONTROL_OF_LANE_1				0x54
 #define HS_TX_CLOCK_LANE_REQUEST_STATE_TIME_CONTROL	0x60
 #define HS_TX_CLOCK_LANE_PREPARE_STATE_TIME_CONTROL	0x61
 #define HS_TX_CLOCK_LANE_HS_ZERO_STATE_TIME_CONTROL	0x62
@@ -137,6 +140,9 @@
 #define HS_TX_DATA_LANE_HS_ZERO_STATE_TIME_CONTROL	0x72
 #define HS_TX_DATA_LANE_TRAIL_STATE_TIME_CONTROL	0x73
 #define HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL		0x74
+#define HS_RX_DATA_LANE_THS_SETTLE_CONTROL		0x75
+#define HS_RX_CONTROL_OF_LANE_2				0x84
+#define HS_RX_CONTROL_OF_LANE_3				0x94
 
 #define DW_MIPI_NEEDS_PHY_CFG_CLK	BIT(0)
 #define DW_MIPI_NEEDS_GRF_CLK		BIT(1)
@@ -171,11 +177,19 @@
 #define RK3399_TXRX_MASTERSLAVEZ	BIT(7)
 #define RK3399_TXRX_ENABLECLK		BIT(6)
 #define RK3399_TXRX_BASEDIR		BIT(5)
+#define RK3399_TXRX_SRC_SEL_ISP0	BIT(4)
+#define RK3399_TXRX_TURNREQUEST		GENMASK(3, 0)
 
 #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
 
 #define to_dsi(nm)	container_of(nm, struct dw_mipi_dsi_rockchip, nm)
 
+enum {
+	DW_DSI_USAGE_IDLE,
+	DW_DSI_USAGE_DSI,
+	DW_DSI_USAGE_PHY,
+};
+
 enum {
 	BANDGAP_97_07,
 	BANDGAP_98_05,
@@ -213,6 +227,10 @@ struct rockchip_dw_dsi_chip_data {
 	u32 lanecfg2_grf_reg;
 	u32 lanecfg2;
 
+	int (*dphy_rx_init)(struct phy *phy);
+	int (*dphy_rx_power_on)(struct phy *phy);
+	int (*dphy_rx_power_off)(struct phy *phy);
+
 	unsigned int flags;
 	unsigned int max_data_lanes;
 };
@@ -236,6 +254,12 @@ struct dw_mipi_dsi_rockchip {
 	struct phy *phy;
 	union phy_configure_opts phy_opts;
 
+	/* being a phy for other mipi hosts */
+	unsigned int usage_mode;
+	struct mutex usage_mutex;
+	struct phy *dphy;
+	struct phy_configure_opts_mipi_dphy dphy_config;
+
 	unsigned int lane_mbps; /* per lane */
 	u16 input_div;
 	u16 feedback_div;
@@ -965,6 +989,17 @@ static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
 	struct device *second;
 	int ret;
 
+	mutex_lock(&dsi->usage_mutex);
+
+	if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
+		DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
+		mutex_unlock(&dsi->usage_mutex);
+		return -EBUSY;
+	}
+
+	dsi->usage_mode = DW_DSI_USAGE_DSI;
+	mutex_unlock(&dsi->usage_mutex);
+
 	ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_ops);
 	if (ret) {
 		DRM_DEV_ERROR(dsi->dev, "Failed to register component: %d\n",
@@ -1000,6 +1035,10 @@ static int dw_mipi_dsi_rockchip_host_detach(void *priv_data,
 
 	component_del(dsi->dev, &dw_mipi_dsi_rockchip_ops);
 
+	mutex_lock(&dsi->usage_mutex);
+	dsi->usage_mode = DW_DSI_USAGE_IDLE;
+	mutex_unlock(&dsi->usage_mutex);
+
 	return 0;
 }
 
@@ -1008,11 +1047,220 @@ static const struct dw_mipi_dsi_host_ops dw_mipi_dsi_rockchip_host_ops = {
 	.detach = dw_mipi_dsi_rockchip_host_detach,
 };
 
+static int dw_mipi_dsi_rockchip_dphy_bind(struct device *dev,
+					  struct device *master,
+					  void *data)
+{
+	/*
+	 * Nothing to do when used as a dphy.
+	 * Just make the rest of Rockchip-DRM happy
+	 * by being here.
+	 */
+
+	return 0;
+}
+
+static void dw_mipi_dsi_rockchip_dphy_unbind(struct device *dev,
+					     struct device *master,
+					     void *data)
+{
+	/* Nothing to do when used as a dphy. */
+}
+
+static const struct component_ops dw_mipi_dsi_rockchip_dphy_ops = {
+	.bind	= dw_mipi_dsi_rockchip_dphy_bind,
+	.unbind	= dw_mipi_dsi_rockchip_dphy_unbind,
+};
+
+static int dw_mipi_dsi_dphy_init(struct phy *phy)
+{
+	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
+	int ret;
+
+	mutex_lock(&dsi->usage_mutex);
+
+	if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
+		DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
+		mutex_unlock(&dsi->usage_mutex);
+		return -EBUSY;
+	}
+
+	dsi->usage_mode = DW_DSI_USAGE_PHY;
+	mutex_unlock(&dsi->usage_mutex);
+
+	ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
+	if (ret < 0)
+		goto err_graph;
+
+	if (dsi->cdata->dphy_rx_init) {
+		ret = clk_prepare_enable(dsi->pclk);
+		if (ret < 0)
+			goto err_init;
+
+		ret = dsi->cdata->dphy_rx_init(phy);
+		clk_disable_unprepare(dsi->pclk);
+		if (ret < 0)
+			goto err_init;
+	}
+
+	return 0;
+
+err_init:
+	component_del(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
+err_graph:
+	mutex_lock(&dsi->usage_mutex);
+	dsi->usage_mode = DW_DSI_USAGE_IDLE;
+	mutex_unlock(&dsi->usage_mutex);
+
+	return ret;
+}
+
+static int dw_mipi_dsi_dphy_exit(struct phy *phy)
+{
+	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
+
+	component_del(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
+
+	mutex_lock(&dsi->usage_mutex);
+	dsi->usage_mode = DW_DSI_USAGE_IDLE;
+	mutex_unlock(&dsi->usage_mutex);
+
+	return 0;
+}
+
+static int dw_mipi_dsi_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+	struct phy_configure_opts_mipi_dphy *config = &opts->mipi_dphy;
+	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
+	int ret;
+
+	ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
+	if (ret)
+		return ret;
+
+	dsi->dphy_config = *config;
+	dsi->lane_mbps = div_u64(config->hs_clk_rate, 1000 * 1000 * 1);
+
+	return 0;
+}
+
+static int dw_mipi_dsi_dphy_power_on(struct phy *phy)
+{
+	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
+	int i, ret;
+
+	DRM_DEV_DEBUG(dsi->dev, "lanes %d - data_rate_mbps %u\n",
+		      dsi->dphy_config.lanes, dsi->lane_mbps);
+
+	i = max_mbps_to_parameter(dsi->lane_mbps);
+	if (i < 0) {
+		DRM_DEV_ERROR(dsi->dev, "failed to get parameter for %dmbps clock\n",
+			      dsi->lane_mbps);
+		return i;
+	}
+
+	ret = pm_runtime_get_sync(dsi->dev);
+	if (ret < 0) {
+		DRM_DEV_ERROR(dsi->dev, "failed to enable device: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(dsi->pclk);
+	if (ret) {
+		DRM_DEV_ERROR(dsi->dev, "Failed to enable pclk: %d\n", ret);
+		goto err_pclk;
+	}
+
+	ret = clk_prepare_enable(dsi->grf_clk);
+	if (ret) {
+		DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
+		goto err_grf_clk;
+	}
+
+	ret = clk_prepare_enable(dsi->phy_cfg_clk);
+	if (ret) {
+		DRM_DEV_ERROR(dsi->dev, "Failed to enable phy_cfg_clk: %d\n", ret);
+		goto err_phy_cfg_clk;
+	}
+
+	/* do soc-variant specific init */
+	if (dsi->cdata->dphy_rx_power_on) {
+		ret = dsi->cdata->dphy_rx_power_on(phy);
+		if (ret < 0) {
+			DRM_DEV_ERROR(dsi->dev, "hardware-specific phy bringup failed: %d\n", ret);
+			goto err_pwr_on;
+		}
+	}
+
+	/*
+	 * Configure hsfreqrange according to frequency values
+	 * Set clock lane and hsfreqrange by lane0(test code 0x44)
+	 */
+	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_CLK, 0);
+	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_0,
+			      HSFREQRANGE_SEL(dppa_map[i].hsfreqrange));
+	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_1, 0);
+	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_2, 0);
+	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_3, 0);
+
+	/* Normal operation */
+	dw_mipi_dsi_phy_write(dsi, 0x0, 0);
+
+	clk_disable_unprepare(dsi->phy_cfg_clk);
+	clk_disable_unprepare(dsi->grf_clk);
+
+	return ret;
+
+err_pwr_on:
+	clk_disable_unprepare(dsi->phy_cfg_clk);
+err_phy_cfg_clk:
+	clk_disable_unprepare(dsi->grf_clk);
+err_grf_clk:
+	clk_disable_unprepare(dsi->pclk);
+err_pclk:
+	pm_runtime_put(dsi->dev);
+	return ret;
+}
+
+static int dw_mipi_dsi_dphy_power_off(struct phy *phy)
+{
+	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
+	int ret;
+
+	ret = clk_prepare_enable(dsi->grf_clk);
+	if (ret) {
+		DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
+		return ret;
+	}
+
+	if (dsi->cdata->dphy_rx_power_off) {
+		ret = dsi->cdata->dphy_rx_power_off(phy);
+		if (ret < 0)
+			DRM_DEV_ERROR(dsi->dev, "hardware-specific phy shutdown failed: %d\n", ret);
+	}
+
+	clk_disable_unprepare(dsi->grf_clk);
+	clk_disable_unprepare(dsi->pclk);
+
+	pm_runtime_put(dsi->dev);
+
+	return ret;
+}
+
+static const struct phy_ops dw_mipi_dsi_dphy_ops = {
+	.configure	= dw_mipi_dsi_dphy_configure,
+	.power_on	= dw_mipi_dsi_dphy_power_on,
+	.power_off	= dw_mipi_dsi_dphy_power_off,
+	.init		= dw_mipi_dsi_dphy_init,
+	.exit		= dw_mipi_dsi_dphy_exit,
+};
+
 static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	struct dw_mipi_dsi_rockchip *dsi;
+	struct phy_provider *phy_provider;
 	struct resource *res;
 	const struct rockchip_dw_dsi_chip_data *cdata =
 				of_device_get_match_data(dev);
@@ -1109,6 +1357,19 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
 	dsi->pdata.priv_data = dsi;
 	platform_set_drvdata(pdev, dsi);
 
+	mutex_init(&dsi->usage_mutex);
+
+	dsi->dphy = devm_phy_create(dev, NULL, &dw_mipi_dsi_dphy_ops);
+	if (IS_ERR(dsi->dphy)) {
+		DRM_DEV_ERROR(&pdev->dev, "failed to create PHY\n");
+		return PTR_ERR(dsi->dphy);
+	}
+
+	phy_set_drvdata(dsi->dphy, dsi);
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
 	dsi->dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
 	if (IS_ERR(dsi->dmd)) {
 		ret = PTR_ERR(dsi->dmd);
@@ -1175,6 +1436,75 @@ static const struct rockchip_dw_dsi_chip_data rk3288_chip_data[] = {
 	{ /* sentinel */ }
 };
 
+static int rk3399_dphy_tx1rx1_init(struct phy *phy)
+{
+	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
+
+	/*
+	 * Set TX1RX1 source to isp1.
+	 * Assume ISP0 is supplied by the RX0 dphy.
+	 */
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
+		     HIWORD_UPDATE(0, RK3399_TXRX_SRC_SEL_ISP0));
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
+		     HIWORD_UPDATE(0, RK3399_TXRX_MASTERSLAVEZ));
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
+		     HIWORD_UPDATE(0, RK3399_TXRX_BASEDIR));
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
+		     HIWORD_UPDATE(0, RK3399_DSI1_ENABLE));
+
+	return 0;
+}
+
+static int rk3399_dphy_tx1rx1_power_on(struct phy *phy)
+{
+	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
+
+	/* tester reset pulse */
+	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_TESTCLR);
+	usleep_range(100, 150);
+
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
+		     HIWORD_UPDATE(0, RK3399_TXRX_MASTERSLAVEZ));
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
+		     HIWORD_UPDATE(RK3399_TXRX_BASEDIR, RK3399_TXRX_BASEDIR));
+
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
+		     HIWORD_UPDATE(0, RK3399_DSI1_FORCERXMODE));
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
+		     HIWORD_UPDATE(0, RK3399_DSI1_FORCETXSTOPMODE));
+
+	/* Disable lane turn around, which is ignored in receive mode */
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
+		     HIWORD_UPDATE(0, RK3399_TXRX_TURNREQUEST));
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
+		     HIWORD_UPDATE(RK3399_DSI1_TURNDISABLE,
+				   RK3399_DSI1_TURNDISABLE));
+	usleep_range(100, 150);
+
+	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
+	usleep_range(100, 150);
+
+	/* Enable dphy lanes */
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
+		     HIWORD_UPDATE(GENMASK(dsi->dphy_config.lanes - 1, 0),
+				   RK3399_DSI1_ENABLE));
+
+	usleep_range(100, 150);
+
+	return 0;
+}
+
+static int rk3399_dphy_tx1rx1_power_off(struct phy *phy)
+{
+	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
+
+	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
+		     HIWORD_UPDATE(0, RK3399_DSI1_ENABLE));
+
+	return 0;
+}
+
 static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
 	{
 		.reg = 0xff960000,
@@ -1217,6 +1547,10 @@ static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
 
 		.flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK,
 		.max_data_lanes = 4,
+
+		.dphy_rx_init = rk3399_dphy_tx1rx1_init,
+		.dphy_rx_power_on = rk3399_dphy_tx1rx1_power_on,
+		.dphy_rx_power_off = rk3399_dphy_tx1rx1_power_off,
 	},
 	{ /* sentinel */ }
 };
-- 
2.29.2

[PATCH 4/6] arm64: dts: rockchip: add #phy-cells to mipi-dsi1

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

The dsi controller includes access to the dphy which might be used
not only for dsi output but also for csi input on dsi1, so add the
necessary #phy-cells to allow it to be used as phy.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index edbbf35fe19e..5d2178cb3e38 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1865,6 +1865,7 @@ mipi_dsi1: mipi@ff968000 {
 		rockchip,grf = <&grf>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		#phy-cells = <0>;
 		status = "disabled";
 
 		ports {
-- 
2.29.2

[PATCH 5/6] arm64: dts: rockchip: add cif clk-control pinctrl for rk3399

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

This enables variant a of the clkout signal for camera applications
and also the cifclkin pinctrl setting.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 5d2178cb3e38..7c661d84df25 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2102,6 +2102,18 @@ clk_32k: clk-32k {
 			};
 		};
 
+		cif {
+			cif_clkin: cif-clkin {
+				rockchip,pins =
+					<2 RK_PB2 3 &pcfg_pull_none>;
+			};
+
+			cif_clkouta: cif-clkouta {
+				rockchip,pins =
+					<2 RK_PB3 3 &pcfg_pull_none>;
+			};
+		};
+
 		edp {
 			edp_hpd: edp-hpd {
 				rockchip,pins =
-- 
2.29.2

[PATCH 6/6] arm64: dts: rockchip: add isp1 node on rk3399

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

ISP1 is supplied by the tx1rx1 dphy, that is controlled from
inside the dsi1 controller, so include the necessary phy-link
for it.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 26 ++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 7c661d84df25..98cec9387300 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1756,6 +1756,32 @@ isp0_mmu: iommu@ff914000 {
 		rockchip,disable-mmu-reset;
 	};
 
+	isp1: isp1@ff920000 {
+		compatible = "rockchip,rk3399-cif-isp";
+		reg = <0x0 0xff920000 0x0 0x4000>;
+		interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru SCLK_ISP1>,
+			 <&cru ACLK_ISP1_WRAPPER>,
+			 <&cru HCLK_ISP1_WRAPPER>;
+		clock-names = "isp", "aclk", "hclk";
+		iommus = <&isp1_mmu>;
+		phys = <&mipi_dsi1>;
+		phy-names = "dphy";
+		power-domains = <&power RK3399_PD_ISP1>;
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
+	};
+
 	isp1_mmu: iommu@ff924000 {
 		compatible = "rockchip,iommu";
 		reg = <0x0 0xff924000 0x0 0x100>, <0x0 0xff925000 0x0 0x100>;
-- 
2.29.2

Re: [PATCH 0/6] Support second Image Signal Processor on rk3399

[Sebastian Fricke]

Hey Heiko,

I have tested your patch set on my nanoPC-T4, here is a complete log
with:
- relevant kernel log entries
- system information
- media ctl output
- sysfs entry information

https://paste.debian.net/1183874/

Additionally, to your patchset I have applied the following patches:
https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup

And just to not cause confusion the `media_dev` entries come from this
unmerged series:
https://patchwork.kernel.org/project/linux-media/list/?series=426269

I have actually been able to stream with both of my cameras at the same
time using the libcamera cam command.
I would like to thank you a lot for making this possible.

If you like to you can add:
Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>

On 02.02.2021 15:56, Heiko Stuebner wrote:
>The rk3399 has two ISPs and right now only the first one is usable.
>The second ISP is connected to the TXRX dphy on the soc.
>
>The phy of ISP1 is only accessible through the DSI controller's
>io-memory, so this series adds support for simply using the dsi
>controller is a phy if needed.
>
>That solution is needed at least on rk3399 and rk3288 but no-one
>has looked at camera support on rk3288 at all, so right now
>only implement the rk3399 specifics.
>
>
>Heiko Stuebner (6):
>  drm/rockchip: dsi: add own additional pclk handling
>  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
>  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
>  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
>  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
>  arm64: dts: rockchip: add isp1 node on rk3399
>
> .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> drivers/gpu/drm/rockchip/Kconfig              |   2 +
> .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> 4 files changed, 384 insertions(+)
>
>-- 
>2.29.2
>

Re: [PATCH 0/6] Support second Image Signal Processor on rk3399

[Heiko Stübner]

Hi Sebastian,

Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
> Hey Heiko,
> 
> I have tested your patch set on my nanoPC-T4, here is a complete log
> with:
> - relevant kernel log entries
> - system information
> - media ctl output
> - sysfs entry information
> 
> https://paste.debian.net/1183874/
> 
> Additionally, to your patchset I have applied the following patches:
> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
> 
> And just to not cause confusion the `media_dev` entries come from this
> unmerged series:
> https://patchwork.kernel.org/project/linux-media/list/?series=426269
> 
> I have actually been able to stream with both of my cameras at the same
> time using the libcamera cam command.
> I would like to thank you a lot for making this possible.

Thanks for testing a dual camera setup. On my board I could only test
the second ISP. And really glad it works for you tool :-) .

Out of curiosity, do you also see that green tint in the images the cameras
produce?

Thanks
Heiko


> If you like to you can add:
> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> 
> On 02.02.2021 15:56, Heiko Stuebner wrote:
> >The rk3399 has two ISPs and right now only the first one is usable.
> >The second ISP is connected to the TXRX dphy on the soc.
> >
> >The phy of ISP1 is only accessible through the DSI controller's
> >io-memory, so this series adds support for simply using the dsi
> >controller is a phy if needed.
> >
> >That solution is needed at least on rk3399 and rk3288 but no-one
> >has looked at camera support on rk3288 at all, so right now
> >only implement the rk3399 specifics.
> >
> >
> >Heiko Stuebner (6):
> >  drm/rockchip: dsi: add own additional pclk handling
> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
> >  arm64: dts: rockchip: add isp1 node on rk3399
> >
> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> > 4 files changed, 384 insertions(+)
> >
> 

Re: [PATCH 0/6] Support second Image Signal Processor on rk3399

[Sebastian Fricke]

Hey Heiko,

On 03.02.2021 20:54, Heiko Stübner wrote:
>Hi Sebastian,
>
>Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
>> Hey Heiko,
>>
>> I have tested your patch set on my nanoPC-T4, here is a complete log
>> with:
>> - relevant kernel log entries
>> - system information
>> - media ctl output
>> - sysfs entry information
>>
>> https://paste.debian.net/1183874/
>>
>> Additionally, to your patchset I have applied the following patches:
>> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
>>
>> And just to not cause confusion the `media_dev` entries come from this
>> unmerged series:
>> https://patchwork.kernel.org/project/linux-media/list/?series=426269
>>
>> I have actually been able to stream with both of my cameras at the same
>> time using the libcamera cam command.
>> I would like to thank you a lot for making this possible.
>
>Thanks for testing a dual camera setup. On my board I could only test
>the second ISP. And really glad it works for you tool :-) .
>
>Out of curiosity, do you also see that green tint in the images the cameras
>produce?

Yes, I do. Actually, I currently have two forms of a green tint, on my
OV13850 everything is quite dark and greenish, which is caused by the
missing 3A algorithms. On my OV4689, I have big patches of the image
with bright green color and flickering, I investigated if this is
connected to the 2nd ISP instance, but that doesn't seem to be the case
as I have the same results when I switch the CSI ports of the cameras.

I have found another issue, while testing I discovered following
issue:
When I start the system with an HDMI monitor connected, then the camera
on the 2nd port doesn't work. This is probably because the RX/TX is
reserved as a TX.
But it made me wonder because if the system has an RX, a TX, and
an RX/TX, why isn't the pure TX used by the monitor and the
cameras take RX and RX/TX?
Or do you think that this is maybe a malfunction of this patch?

>
>Thanks
>Heiko

Greetings,
Sebastian

>
>
>> If you like to you can add:
>> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
>>
>> On 02.02.2021 15:56, Heiko Stuebner wrote:
>> >The rk3399 has two ISPs and right now only the first one is usable.
>> >The second ISP is connected to the TXRX dphy on the soc.
>> >
>> >The phy of ISP1 is only accessible through the DSI controller's
>> >io-memory, so this series adds support for simply using the dsi
>> >controller is a phy if needed.
>> >
>> >That solution is needed at least on rk3399 and rk3288 but no-one
>> >has looked at camera support on rk3288 at all, so right now
>> >only implement the rk3399 specifics.
>> >
>> >
>> >Heiko Stuebner (6):
>> >  drm/rockchip: dsi: add own additional pclk handling
>> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
>> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
>> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
>> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
>> >  arm64: dts: rockchip: add isp1 node on rk3399
>> >
>> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
>> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
>> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
>> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
>> > 4 files changed, 384 insertions(+)
>> >
>>
>
>
>
>

Re: [PATCH 0/6] Support second Image Signal Processor on rk3399

[Heiko Stübner]

Hi Sebastian,

Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
> On 03.02.2021 20:54, Heiko Stübner wrote:
> >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
> >> I have tested your patch set on my nanoPC-T4, here is a complete log
> >> with:
> >> - relevant kernel log entries
> >> - system information
> >> - media ctl output
> >> - sysfs entry information
> >>
> >> https://paste.debian.net/1183874/
> >>
> >> Additionally, to your patchset I have applied the following patches:
> >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
> >>
> >> And just to not cause confusion the `media_dev` entries come from this
> >> unmerged series:
> >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
> >>
> >> I have actually been able to stream with both of my cameras at the same
> >> time using the libcamera cam command.
> >> I would like to thank you a lot for making this possible.
> >
> >Thanks for testing a dual camera setup. On my board I could only test
> >the second ISP. And really glad it works for you tool :-) .
> >
> >Out of curiosity, do you also see that green tint in the images the cameras
> >produce?
> 
> Yes, I do. Actually, I currently have two forms of a green tint, on my
> OV13850 everything is quite dark and greenish, which is caused by the
> missing 3A algorithms. On my OV4689, I have big patches of the image
> with bright green color and flickering, I investigated if this is
> connected to the 2nd ISP instance, but that doesn't seem to be the case
> as I have the same results when I switch the CSI ports of the cameras.
> 
> I have found another issue, while testing I discovered following
> issue:
> When I start the system with an HDMI monitor connected, then the camera
> on the 2nd port doesn't work. This is probably because the RX/TX is
> reserved as a TX.
> But it made me wonder because if the system has an RX, a TX, and
> an RX/TX, why isn't the pure TX used by the monitor and the
> cameras take RX and RX/TX?
> Or do you think that this is maybe a malfunction of this patch?

I don't think it is an issue with this specific series, but still puzzling.

I.e. the DPHYs are actually only relevant to the DSI controllers,
with TX0 being connected to DSI0 and TX1RX1 being connected
to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.

Out of curiosity what happens, when you boot without hdmi connected
turn on the cameras, connect the hdmi after this, try the cameras again?


Heiko

> 
> >
> >Thanks
> >Heiko
> 
> Greetings,
> Sebastian
> 
> >
> >
> >> If you like to you can add:
> >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> >>
> >> On 02.02.2021 15:56, Heiko Stuebner wrote:
> >> >The rk3399 has two ISPs and right now only the first one is usable.
> >> >The second ISP is connected to the TXRX dphy on the soc.
> >> >
> >> >The phy of ISP1 is only accessible through the DSI controller's
> >> >io-memory, so this series adds support for simply using the dsi
> >> >controller is a phy if needed.
> >> >
> >> >That solution is needed at least on rk3399 and rk3288 but no-one
> >> >has looked at camera support on rk3288 at all, so right now
> >> >only implement the rk3399 specifics.
> >> >
> >> >
> >> >Heiko Stuebner (6):
> >> >  drm/rockchip: dsi: add own additional pclk handling
> >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
> >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
> >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
> >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
> >> >  arm64: dts: rockchip: add isp1 node on rk3399
> >> >
> >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
> >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> >> > 4 files changed, 384 insertions(+)
> >> >
> >>
> >
> >
> >
> >
> 

Re: [PATCH 0/6] Support second Image Signal Processor on rk3399

[Heiko Stübner]

Hi Sebastian,

I did some tests myself today as well and can confirm your
hdmi related finding - at least when plugged in on boot.

I tried some combinations of camera vs. hdmi and it seems
really only when hdmi is plugged in on boot

(1)
- boot
- camera
--> works

(2)
- boot
- camera
- hdmi plugged in
- hdmi works
- camera
--> works

(3)
- hdmi plugged in
- boot
- hdmi works
- camera
--> camera doesn't work

(4)
- boot
- hdmi plugged in
- hdmi works
- camera
-> camera works


With a bit of brute-force [0] it seems the camera also works again even
with hdmi connected on boot. So conclusion would be that some clock
is misbehaving.

Now we'll "only" need to find out which one that is.


Heiko


[0]
Don't disable any clock gates

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 070dc47e95a1..8daf1fc3388c 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
 
        set ^= enable;
 
+if (!enable)
+return;
+
        if (gate->lock)
                spin_lock_irqsave(gate->lock, flags);
        else



Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner:
> Hi Sebastian,
> 
> Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
> > On 03.02.2021 20:54, Heiko Stübner wrote:
> > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
> > >> I have tested your patch set on my nanoPC-T4, here is a complete log
> > >> with:
> > >> - relevant kernel log entries
> > >> - system information
> > >> - media ctl output
> > >> - sysfs entry information
> > >>
> > >> https://paste.debian.net/1183874/
> > >>
> > >> Additionally, to your patchset I have applied the following patches:
> > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
> > >>
> > >> And just to not cause confusion the `media_dev` entries come from this
> > >> unmerged series:
> > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
> > >>
> > >> I have actually been able to stream with both of my cameras at the same
> > >> time using the libcamera cam command.
> > >> I would like to thank you a lot for making this possible.
> > >
> > >Thanks for testing a dual camera setup. On my board I could only test
> > >the second ISP. And really glad it works for you tool :-) .
> > >
> > >Out of curiosity, do you also see that green tint in the images the cameras
> > >produce?
> > 
> > Yes, I do. Actually, I currently have two forms of a green tint, on my
> > OV13850 everything is quite dark and greenish, which is caused by the
> > missing 3A algorithms. On my OV4689, I have big patches of the image
> > with bright green color and flickering, I investigated if this is
> > connected to the 2nd ISP instance, but that doesn't seem to be the case
> > as I have the same results when I switch the CSI ports of the cameras.
> > 
> > I have found another issue, while testing I discovered following
> > issue:
> > When I start the system with an HDMI monitor connected, then the camera
> > on the 2nd port doesn't work. This is probably because the RX/TX is
> > reserved as a TX.
> > But it made me wonder because if the system has an RX, a TX, and
> > an RX/TX, why isn't the pure TX used by the monitor and the
> > cameras take RX and RX/TX?
> > Or do you think that this is maybe a malfunction of this patch?
> 
> I don't think it is an issue with this specific series, but still puzzling.
> 
> I.e. the DPHYs are actually only relevant to the DSI controllers,
> with TX0 being connected to DSI0 and TX1RX1 being connected
> to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.
> 
> Out of curiosity what happens, when you boot without hdmi connected
> turn on the cameras, connect the hdmi after this, try the cameras again?
> 
> 
> Heiko
> 
> > 
> > >
> > >Thanks
> > >Heiko
> > 
> > Greetings,
> > Sebastian
> > 
> > >
> > >
> > >> If you like to you can add:
> > >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> > >>
> > >> On 02.02.2021 15:56, Heiko Stuebner wrote:
> > >> >The rk3399 has two ISPs and right now only the first one is usable.
> > >> >The second ISP is connected to the TXRX dphy on the soc.
> > >> >
> > >> >The phy of ISP1 is only accessible through the DSI controller's
> > >> >io-memory, so this series adds support for simply using the dsi
> > >> >controller is a phy if needed.
> > >> >
> > >> >That solution is needed at least on rk3399 and rk3288 but no-one
> > >> >has looked at camera support on rk3288 at all, so right now
> > >> >only implement the rk3399 specifics.
> > >> >
> > >> >
> > >> >Heiko Stuebner (6):
> > >> >  drm/rockchip: dsi: add own additional pclk handling
> > >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
> > >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
> > >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
> > >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
> > >> >  arm64: dts: rockchip: add isp1 node on rk3399
> > >> >
> > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> > >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
> > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> > >> > 4 files changed, 384 insertions(+)
> > >> >
> > >>
> > >
> > >
> > >
> > >
> > 
> 
> 

Re: [PATCH 2/6] dt-bindings: display: rockchip-dsi: add optional #phy-cells property

[Rob Herring]

On Tue, 02 Feb 2021 15:56:28 +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> The Rockchip DSI controller on some SoCs also controls a bidrectional
> dphy, which would be connected to an Image Signal Processor as a phy
> in the rx configuration.
> 
> So allow a #phy-cells property for the dsi controller.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
>  .../bindings/display/rockchip/dw_mipi_dsi_rockchip.txt           | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 0/6] Support second Image Signal Processor on rk3399

[Heiko Stübner]

Hi Sebastian,

Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner:
> Hi Sebastian,
> 
> I did some tests myself today as well and can confirm your
> hdmi related finding - at least when plugged in on boot.
> 
> I tried some combinations of camera vs. hdmi and it seems
> really only when hdmi is plugged in on boot

as you can see in v2, it should work now even with hdmi
connected on boot. My patch ignored the grf-clock when
doing the grf-based init.

All clocks are on during boot and I guess the hdmi-driver
did disable it after its probe. The phy_power_on functions
did handle it correctly already, so it was only happening
with hdmi connected on boot.


Btw. do you plan on submitting your ov13850 driver
soonish?


Heiko


> 
> (1)
> - boot
> - camera
> --> works
> 
> (2)
> - boot
> - camera
> - hdmi plugged in
> - hdmi works
> - camera
> --> works
> 
> (3)
> - hdmi plugged in
> - boot
> - hdmi works
> - camera
> --> camera doesn't work
> 
> (4)
> - boot
> - hdmi plugged in
> - hdmi works
> - camera
> -> camera works
> 
> 
> With a bit of brute-force [0] it seems the camera also works again even
> with hdmi connected on boot. So conclusion would be that some clock
> is misbehaving.
> 
> Now we'll "only" need to find out which one that is.
> 
> 
> Heiko
> 
> 
> [0]
> Don't disable any clock gates
> 
> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> index 070dc47e95a1..8daf1fc3388c 100644
> --- a/drivers/clk/clk-gate.c
> +++ b/drivers/clk/clk-gate.c
> @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
>  
>         set ^= enable;
>  
> +if (!enable)
> +return;
> +
>         if (gate->lock)
>                 spin_lock_irqsave(gate->lock, flags);
>         else
> 
> 
> 
> Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner:
> > Hi Sebastian,
> > 
> > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
> > > On 03.02.2021 20:54, Heiko Stübner wrote:
> > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
> > > >> I have tested your patch set on my nanoPC-T4, here is a complete log
> > > >> with:
> > > >> - relevant kernel log entries
> > > >> - system information
> > > >> - media ctl output
> > > >> - sysfs entry information
> > > >>
> > > >> https://paste.debian.net/1183874/
> > > >>
> > > >> Additionally, to your patchset I have applied the following patches:
> > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
> > > >>
> > > >> And just to not cause confusion the `media_dev` entries come from this
> > > >> unmerged series:
> > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
> > > >>
> > > >> I have actually been able to stream with both of my cameras at the same
> > > >> time using the libcamera cam command.
> > > >> I would like to thank you a lot for making this possible.
> > > >
> > > >Thanks for testing a dual camera setup. On my board I could only test
> > > >the second ISP. And really glad it works for you tool :-) .
> > > >
> > > >Out of curiosity, do you also see that green tint in the images the cameras
> > > >produce?
> > > 
> > > Yes, I do. Actually, I currently have two forms of a green tint, on my
> > > OV13850 everything is quite dark and greenish, which is caused by the
> > > missing 3A algorithms. On my OV4689, I have big patches of the image
> > > with bright green color and flickering, I investigated if this is
> > > connected to the 2nd ISP instance, but that doesn't seem to be the case
> > > as I have the same results when I switch the CSI ports of the cameras.
> > > 
> > > I have found another issue, while testing I discovered following
> > > issue:
> > > When I start the system with an HDMI monitor connected, then the camera
> > > on the 2nd port doesn't work. This is probably because the RX/TX is
> > > reserved as a TX.
> > > But it made me wonder because if the system has an RX, a TX, and
> > > an RX/TX, why isn't the pure TX used by the monitor and the
> > > cameras take RX and RX/TX?
> > > Or do you think that this is maybe a malfunction of this patch?
> > 
> > I don't think it is an issue with this specific series, but still puzzling.
> > 
> > I.e. the DPHYs are actually only relevant to the DSI controllers,
> > with TX0 being connected to DSI0 and TX1RX1 being connected
> > to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.
> > 
> > Out of curiosity what happens, when you boot without hdmi connected
> > turn on the cameras, connect the hdmi after this, try the cameras again?
> > 
> > 
> > Heiko
> > 
> > > 
> > > >
> > > >Thanks
> > > >Heiko
> > > 
> > > Greetings,
> > > Sebastian
> > > 
> > > >
> > > >
> > > >> If you like to you can add:
> > > >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> > > >>
> > > >> On 02.02.2021 15:56, Heiko Stuebner wrote:
> > > >> >The rk3399 has two ISPs and right now only the first one is usable.
> > > >> >The second ISP is connected to the TXRX dphy on the soc.
> > > >> >
> > > >> >The phy of ISP1 is only accessible through the DSI controller's
> > > >> >io-memory, so this series adds support for simply using the dsi
> > > >> >controller is a phy if needed.
> > > >> >
> > > >> >That solution is needed at least on rk3399 and rk3288 but no-one
> > > >> >has looked at camera support on rk3288 at all, so right now
> > > >> >only implement the rk3399 specifics.
> > > >> >
> > > >> >
> > > >> >Heiko Stuebner (6):
> > > >> >  drm/rockchip: dsi: add own additional pclk handling
> > > >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
> > > >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
> > > >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
> > > >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
> > > >> >  arm64: dts: rockchip: add isp1 node on rk3399
> > > >> >
> > > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> > > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> > > >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
> > > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> > > >> > 4 files changed, 384 insertions(+)
> > > >> >
> > > >>
> > > >
> > > >
> > > >
> > > >
> > > 
> > 
> > 
> 
> 

Re: [PATCH 0/6] Support second Image Signal Processor on rk3399

[Sebastian Fricke]

Hey Heiko,

On 10.02.2021 12:15, Heiko Stübner wrote:
>Hi Sebastian,
>
>Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner:
>> Hi Sebastian,
>>
>> I did some tests myself today as well and can confirm your
>> hdmi related finding - at least when plugged in on boot.
>>
>> I tried some combinations of camera vs. hdmi and it seems
>> really only when hdmi is plugged in on boot
>
>as you can see in v2, it should work now even with hdmi
>connected on boot. My patch ignored the grf-clock when
>doing the grf-based init.
>
>All clocks are on during boot and I guess the hdmi-driver
>did disable it after its probe. The phy_power_on functions
>did handle it correctly already, so it was only happening
>with hdmi connected on boot.

Thank you very much for solving that problem, I've tested the scenarios
described below and it works like a charm. (With your V2)
>
>
>Btw. do you plan on submitting your ov13850 driver
>soonish?

Actually, I have posted the patch already see here:
https://patchwork.kernel.org/project/linux-media/patch/20210130182313.32903-2-sebastian.fricke@posteo.net/

I currently review the requested changes and questions and will soon
post a second version, but I expect quite some time until it is actually
merged.

>
>
>Heiko

Greetings,
Sebastian

>
>
>>
>> (1)
>> - boot
>> - camera
>> --> works
>>
>> (2)
>> - boot
>> - camera
>> - hdmi plugged in
>> - hdmi works
>> - camera
>> --> works
>>
>> (3)
>> - hdmi plugged in
>> - boot
>> - hdmi works
>> - camera
>> --> camera doesn't work
>>
>> (4)
>> - boot
>> - hdmi plugged in
>> - hdmi works
>> - camera
>> -> camera works
>>
>>
>> With a bit of brute-force [0] it seems the camera also works again even
>> with hdmi connected on boot. So conclusion would be that some clock
>> is misbehaving.
>>
>> Now we'll "only" need to find out which one that is.
>>
>>
>> Heiko
>>
>>
>> [0]
>> Don't disable any clock gates
>>
>> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
>> index 070dc47e95a1..8daf1fc3388c 100644
>> --- a/drivers/clk/clk-gate.c
>> +++ b/drivers/clk/clk-gate.c
>> @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
>>
>>         set ^= enable;
>>
>> +if (!enable)
>> +return;
>> +
>>         if (gate->lock)
>>                 spin_lock_irqsave(gate->lock, flags);
>>         else
>>
>>
>>
>> Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner:
>> > Hi Sebastian,
>> >
>> > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
>> > > On 03.02.2021 20:54, Heiko Stübner wrote:
>> > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
>> > > >> I have tested your patch set on my nanoPC-T4, here is a complete log
>> > > >> with:
>> > > >> - relevant kernel log entries
>> > > >> - system information
>> > > >> - media ctl output
>> > > >> - sysfs entry information
>> > > >>
>> > > >> https://paste.debian.net/1183874/
>> > > >>
>> > > >> Additionally, to your patchset I have applied the following patches:
>> > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
>> > > >>
>> > > >> And just to not cause confusion the `media_dev` entries come from this
>> > > >> unmerged series:
>> > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
>> > > >>
>> > > >> I have actually been able to stream with both of my cameras at the same
>> > > >> time using the libcamera cam command.
>> > > >> I would like to thank you a lot for making this possible.
>> > > >
>> > > >Thanks for testing a dual camera setup. On my board I could only test
>> > > >the second ISP. And really glad it works for you tool :-) .
>> > > >
>> > > >Out of curiosity, do you also see that green tint in the images the cameras
>> > > >produce?
>> > >
>> > > Yes, I do. Actually, I currently have two forms of a green tint, on my
>> > > OV13850 everything is quite dark and greenish, which is caused by the
>> > > missing 3A algorithms. On my OV4689, I have big patches of the image
>> > > with bright green color and flickering, I investigated if this is
>> > > connected to the 2nd ISP instance, but that doesn't seem to be the case
>> > > as I have the same results when I switch the CSI ports of the cameras.
>> > >
>> > > I have found another issue, while testing I discovered following
>> > > issue:
>> > > When I start the system with an HDMI monitor connected, then the camera
>> > > on the 2nd port doesn't work. This is probably because the RX/TX is
>> > > reserved as a TX.
>> > > But it made me wonder because if the system has an RX, a TX, and
>> > > an RX/TX, why isn't the pure TX used by the monitor and the
>> > > cameras take RX and RX/TX?
>> > > Or do you think that this is maybe a malfunction of this patch?
>> >
>> > I don't think it is an issue with this specific series, but still puzzling.
>> >
>> > I.e. the DPHYs are actually only relevant to the DSI controllers,
>> > with TX0 being connected to DSI0 and TX1RX1 being connected
>> > to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.
>> >
>> > Out of curiosity what happens, when you boot without hdmi connected
>> > turn on the cameras, connect the hdmi after this, try the cameras again?
>> >
>> >
>> > Heiko
>> >
>> > >
>> > > >
>> > > >Thanks
>> > > >Heiko
>> > >
>> > > Greetings,
>> > > Sebastian
>> > >
>> > > >
>> > > >
>> > > >> If you like to you can add:
>> > > >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
>> > > >>
>> > > >> On 02.02.2021 15:56, Heiko Stuebner wrote:
>> > > >> >The rk3399 has two ISPs and right now only the first one is usable.
>> > > >> >The second ISP is connected to the TXRX dphy on the soc.
>> > > >> >
>> > > >> >The phy of ISP1 is only accessible through the DSI controller's
>> > > >> >io-memory, so this series adds support for simply using the dsi
>> > > >> >controller is a phy if needed.
>> > > >> >
>> > > >> >That solution is needed at least on rk3399 and rk3288 but no-one
>> > > >> >has looked at camera support on rk3288 at all, so right now
>> > > >> >only implement the rk3399 specifics.
>> > > >> >
>> > > >> >
>> > > >> >Heiko Stuebner (6):
>> > > >> >  drm/rockchip: dsi: add own additional pclk handling
>> > > >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
>> > > >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
>> > > >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
>> > > >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
>> > > >> >  arm64: dts: rockchip: add isp1 node on rk3399
>> > > >> >
>> > > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
>> > > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
>> > > >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
>> > > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
>> > > >> > 4 files changed, 384 insertions(+)
>> > > >> >
>> > > >>
>> > > >
>> > > >
>> > > >
>> > > >
>> > >
>> >
>> >
>>
>>
>
>
>
>

Re: [PATCH 0/6] Support second Image Signal Processor on rk3399

[Heiko Stübner]

Hi Sebastian,

Am Donnerstag, 11. Februar 2021, 06:25:15 CET schrieb Sebastian Fricke:
> Hey Heiko,
> 
> On 10.02.2021 12:15, Heiko Stübner wrote:
> >Hi Sebastian,
> >
> >Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner:
> >> Hi Sebastian,
> >>
> >> I did some tests myself today as well and can confirm your
> >> hdmi related finding - at least when plugged in on boot.
> >>
> >> I tried some combinations of camera vs. hdmi and it seems
> >> really only when hdmi is plugged in on boot
> >
> >as you can see in v2, it should work now even with hdmi
> >connected on boot. My patch ignored the grf-clock when
> >doing the grf-based init.
> >
> >All clocks are on during boot and I guess the hdmi-driver
> >did disable it after its probe. The phy_power_on functions
> >did handle it correctly already, so it was only happening
> >with hdmi connected on boot.
> 
> Thank you very much for solving that problem, I've tested the scenarios
> described below and it works like a charm. (With your V2)
> >
> >
> >Btw. do you plan on submitting your ov13850 driver
> >soonish?
> 
> Actually, I have posted the patch already see here:
> https://patchwork.kernel.org/project/linux-media/patch/20210130182313.32903-2-sebastian.fricke@posteo.net/

very cool to see

> I currently review the requested changes and questions and will soon
> post a second version, but I expect quite some time until it is actually
> merged.

could you Cc me on future versions?


Thanks
Heiko
> 
> Greetings,
> Sebastian
> 
> >
> >
> >>
> >> (1)
> >> - boot
> >> - camera
> >> --> works
> >>
> >> (2)
> >> - boot
> >> - camera
> >> - hdmi plugged in
> >> - hdmi works
> >> - camera
> >> --> works
> >>
> >> (3)
> >> - hdmi plugged in
> >> - boot
> >> - hdmi works
> >> - camera
> >> --> camera doesn't work
> >>
> >> (4)
> >> - boot
> >> - hdmi plugged in
> >> - hdmi works
> >> - camera
> >> -> camera works
> >>
> >>
> >> With a bit of brute-force [0] it seems the camera also works again even
> >> with hdmi connected on boot. So conclusion would be that some clock
> >> is misbehaving.
> >>
> >> Now we'll "only" need to find out which one that is.
> >>
> >>
> >> Heiko
> >>
> >>
> >> [0]
> >> Don't disable any clock gates
> >>
> >> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> >> index 070dc47e95a1..8daf1fc3388c 100644
> >> --- a/drivers/clk/clk-gate.c
> >> +++ b/drivers/clk/clk-gate.c
> >> @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
> >>
> >>         set ^= enable;
> >>
> >> +if (!enable)
> >> +return;
> >> +
> >>         if (gate->lock)
> >>                 spin_lock_irqsave(gate->lock, flags);
> >>         else
> >>
> >>
> >>
> >> Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner:
> >> > Hi Sebastian,
> >> >
> >> > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
> >> > > On 03.02.2021 20:54, Heiko Stübner wrote:
> >> > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
> >> > > >> I have tested your patch set on my nanoPC-T4, here is a complete log
> >> > > >> with:
> >> > > >> - relevant kernel log entries
> >> > > >> - system information
> >> > > >> - media ctl output
> >> > > >> - sysfs entry information
> >> > > >>
> >> > > >> https://paste.debian.net/1183874/
> >> > > >>
> >> > > >> Additionally, to your patchset I have applied the following patches:
> >> > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
> >> > > >>
> >> > > >> And just to not cause confusion the `media_dev` entries come from this
> >> > > >> unmerged series:
> >> > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
> >> > > >>
> >> > > >> I have actually been able to stream with both of my cameras at the same
> >> > > >> time using the libcamera cam command.
> >> > > >> I would like to thank you a lot for making this possible.
> >> > > >
> >> > > >Thanks for testing a dual camera setup. On my board I could only test
> >> > > >the second ISP. And really glad it works for you tool :-) .
> >> > > >
> >> > > >Out of curiosity, do you also see that green tint in the images the cameras
> >> > > >produce?
> >> > >
> >> > > Yes, I do. Actually, I currently have two forms of a green tint, on my
> >> > > OV13850 everything is quite dark and greenish, which is caused by the
> >> > > missing 3A algorithms. On my OV4689, I have big patches of the image
> >> > > with bright green color and flickering, I investigated if this is
> >> > > connected to the 2nd ISP instance, but that doesn't seem to be the case
> >> > > as I have the same results when I switch the CSI ports of the cameras.
> >> > >
> >> > > I have found another issue, while testing I discovered following
> >> > > issue:
> >> > > When I start the system with an HDMI monitor connected, then the camera
> >> > > on the 2nd port doesn't work. This is probably because the RX/TX is
> >> > > reserved as a TX.
> >> > > But it made me wonder because if the system has an RX, a TX, and
> >> > > an RX/TX, why isn't the pure TX used by the monitor and the
> >> > > cameras take RX and RX/TX?
> >> > > Or do you think that this is maybe a malfunction of this patch?
> >> >
> >> > I don't think it is an issue with this specific series, but still puzzling.
> >> >
> >> > I.e. the DPHYs are actually only relevant to the DSI controllers,
> >> > with TX0 being connected to DSI0 and TX1RX1 being connected
> >> > to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.
> >> >
> >> > Out of curiosity what happens, when you boot without hdmi connected
> >> > turn on the cameras, connect the hdmi after this, try the cameras again?
> >> >
> >> >
> >> > Heiko
> >> >
> >> > >
> >> > > >
> >> > > >Thanks
> >> > > >Heiko
> >> > >
> >> > > Greetings,
> >> > > Sebastian
> >> > >
> >> > > >
> >> > > >
> >> > > >> If you like to you can add:
> >> > > >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> >> > > >>
> >> > > >> On 02.02.2021 15:56, Heiko Stuebner wrote:
> >> > > >> >The rk3399 has two ISPs and right now only the first one is usable.
> >> > > >> >The second ISP is connected to the TXRX dphy on the soc.
> >> > > >> >
> >> > > >> >The phy of ISP1 is only accessible through the DSI controller's
> >> > > >> >io-memory, so this series adds support for simply using the dsi
> >> > > >> >controller is a phy if needed.
> >> > > >> >
> >> > > >> >That solution is needed at least on rk3399 and rk3288 but no-one
> >> > > >> >has looked at camera support on rk3288 at all, so right now
> >> > > >> >only implement the rk3399 specifics.
> >> > > >> >
> >> > > >> >
> >> > > >> >Heiko Stuebner (6):
> >> > > >> >  drm/rockchip: dsi: add own additional pclk handling
> >> > > >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
> >> > > >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
> >> > > >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
> >> > > >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
> >> > > >> >  arm64: dts: rockchip: add isp1 node on rk3399
> >> > > >> >
> >> > > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> >> > > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> >> > > >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
> >> > > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> >> > > >> > 4 files changed, 384 insertions(+)
> >> > > >> >
> >> > > >>
> >> > > >
> >> > > >
> >> > > >
> >> > > >
> >> > >
> >> >
> >> >
> >>
> >>
> >
> >
> >
> >
> 

Re: [PATCH 0/6] Support second Image Signal Processor on rk3399

[Sebastian Fricke]

Hey Heiko,

On 11.02.2021 15:42, Heiko Stübner wrote:
>Hi Sebastian,
>
>Am Donnerstag, 11. Februar 2021, 06:25:15 CET schrieb Sebastian Fricke:
>> Hey Heiko,
>>
>> On 10.02.2021 12:15, Heiko Stübner wrote:
>> >Hi Sebastian,
>> >
>> >Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner:
>> >> Hi Sebastian,
>> >>
>> >> I did some tests myself today as well and can confirm your
>> >> hdmi related finding - at least when plugged in on boot.
>> >>
>> >> I tried some combinations of camera vs. hdmi and it seems
>> >> really only when hdmi is plugged in on boot
>> >
>> >as you can see in v2, it should work now even with hdmi
>> >connected on boot. My patch ignored the grf-clock when
>> >doing the grf-based init.
>> >
>> >All clocks are on during boot and I guess the hdmi-driver
>> >did disable it after its probe. The phy_power_on functions
>> >did handle it correctly already, so it was only happening
>> >with hdmi connected on boot.
>>
>> Thank you very much for solving that problem, I've tested the scenarios
>> described below and it works like a charm. (With your V2)
>> >
>> >
>> >Btw. do you plan on submitting your ov13850 driver
>> >soonish?
>>
>> Actually, I have posted the patch already see here:
>> https://patchwork.kernel.org/project/linux-media/patch/20210130182313.32903-2-sebastian.fricke@posteo.net/
>
>very cool to see
>
>> I currently review the requested changes and questions and will soon
>> post a second version, but I expect quite some time until it is actually
>> merged.
>
>could you Cc me on future versions?

Sure will do :)

>
>
>Thanks
>Heiko

Sebastian
>>
>> Greetings,
>> Sebastian
>>
>> >
>> >
>> >>
>> >> (1)
>> >> - boot
>> >> - camera
>> >> --> works
>> >>
>> >> (2)
>> >> - boot
>> >> - camera
>> >> - hdmi plugged in
>> >> - hdmi works
>> >> - camera
>> >> --> works
>> >>
>> >> (3)
>> >> - hdmi plugged in
>> >> - boot
>> >> - hdmi works
>> >> - camera
>> >> --> camera doesn't work
>> >>
>> >> (4)
>> >> - boot
>> >> - hdmi plugged in
>> >> - hdmi works
>> >> - camera
>> >> -> camera works
>> >>
>> >>
>> >> With a bit of brute-force [0] it seems the camera also works again even
>> >> with hdmi connected on boot. So conclusion would be that some clock
>> >> is misbehaving.
>> >>
>> >> Now we'll "only" need to find out which one that is.
>> >>
>> >>
>> >> Heiko
>> >>
>> >>
>> >> [0]
>> >> Don't disable any clock gates
>> >>
>> >> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
>> >> index 070dc47e95a1..8daf1fc3388c 100644
>> >> --- a/drivers/clk/clk-gate.c
>> >> +++ b/drivers/clk/clk-gate.c
>> >> @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
>> >>
>> >>         set ^= enable;
>> >>
>> >> +if (!enable)
>> >> +return;
>> >> +
>> >>         if (gate->lock)
>> >>                 spin_lock_irqsave(gate->lock, flags);
>> >>         else
>> >>
>> >>
>> >>
>> >> Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner:
>> >> > Hi Sebastian,
>> >> >
>> >> > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
>> >> > > On 03.02.2021 20:54, Heiko Stübner wrote:
>> >> > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
>> >> > > >> I have tested your patch set on my nanoPC-T4, here is a complete log
>> >> > > >> with:
>> >> > > >> - relevant kernel log entries
>> >> > > >> - system information
>> >> > > >> - media ctl output
>> >> > > >> - sysfs entry information
>> >> > > >>
>> >> > > >> https://paste.debian.net/1183874/
>> >> > > >>
>> >> > > >> Additionally, to your patchset I have applied the following patches:
>> >> > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
>> >> > > >>
>> >> > > >> And just to not cause confusion the `media_dev` entries come from this
>> >> > > >> unmerged series:
>> >> > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
>> >> > > >>
>> >> > > >> I have actually been able to stream with both of my cameras at the same
>> >> > > >> time using the libcamera cam command.
>> >> > > >> I would like to thank you a lot for making this possible.
>> >> > > >
>> >> > > >Thanks for testing a dual camera setup. On my board I could only test
>> >> > > >the second ISP. And really glad it works for you tool :-) .
>> >> > > >
>> >> > > >Out of curiosity, do you also see that green tint in the images the cameras
>> >> > > >produce?
>> >> > >
>> >> > > Yes, I do. Actually, I currently have two forms of a green tint, on my
>> >> > > OV13850 everything is quite dark and greenish, which is caused by the
>> >> > > missing 3A algorithms. On my OV4689, I have big patches of the image
>> >> > > with bright green color and flickering, I investigated if this is
>> >> > > connected to the 2nd ISP instance, but that doesn't seem to be the case
>> >> > > as I have the same results when I switch the CSI ports of the cameras.
>> >> > >
>> >> > > I have found another issue, while testing I discovered following
>> >> > > issue:
>> >> > > When I start the system with an HDMI monitor connected, then the camera
>> >> > > on the 2nd port doesn't work. This is probably because the RX/TX is
>> >> > > reserved as a TX.
>> >> > > But it made me wonder because if the system has an RX, a TX, and
>> >> > > an RX/TX, why isn't the pure TX used by the monitor and the
>> >> > > cameras take RX and RX/TX?
>> >> > > Or do you think that this is maybe a malfunction of this patch?
>> >> >
>> >> > I don't think it is an issue with this specific series, but still puzzling.
>> >> >
>> >> > I.e. the DPHYs are actually only relevant to the DSI controllers,
>> >> > with TX0 being connected to DSI0 and TX1RX1 being connected
>> >> > to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.
>> >> >
>> >> > Out of curiosity what happens, when you boot without hdmi connected
>> >> > turn on the cameras, connect the hdmi after this, try the cameras again?
>> >> >
>> >> >
>> >> > Heiko
>> >> >
>> >> > >
>> >> > > >
>> >> > > >Thanks
>> >> > > >Heiko
>> >> > >
>> >> > > Greetings,
>> >> > > Sebastian
>> >> > >
>> >> > > >
>> >> > > >
>> >> > > >> If you like to you can add:
>> >> > > >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
>> >> > > >>
>> >> > > >> On 02.02.2021 15:56, Heiko Stuebner wrote:
>> >> > > >> >The rk3399 has two ISPs and right now only the first one is usable.
>> >> > > >> >The second ISP is connected to the TXRX dphy on the soc.
>> >> > > >> >
>> >> > > >> >The phy of ISP1 is only accessible through the DSI controller's
>> >> > > >> >io-memory, so this series adds support for simply using the dsi
>> >> > > >> >controller is a phy if needed.
>> >> > > >> >
>> >> > > >> >That solution is needed at least on rk3399 and rk3288 but no-one
>> >> > > >> >has looked at camera support on rk3288 at all, so right now
>> >> > > >> >only implement the rk3399 specifics.
>> >> > > >> >
>> >> > > >> >
>> >> > > >> >Heiko Stuebner (6):
>> >> > > >> >  drm/rockchip: dsi: add own additional pclk handling
>> >> > > >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
>> >> > > >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
>> >> > > >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
>> >> > > >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
>> >> > > >> >  arm64: dts: rockchip: add isp1 node on rk3399
>> >> > > >> >
>> >> > > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
>> >> > > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
>> >> > > >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
>> >> > > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
>> >> > > >> > 4 files changed, 384 insertions(+)
>> >> > > >> >
>> >> > > >>
>> >> > > >
>> >> > > >
>> >> > > >
>> >> > > >
>> >> > >
>> >> >
>> >> >
>> >>
>> >>
>> >
>> >
>> >
>> >
>>
>
>
>
>

Re: [PATCH 0/6] Support second Image Signal Processor on rk3399

[Heiko Stuebner]

Hi Sebastian,

Am Samstag, 13. Februar 2021, 12:19:57 CET schrieb Sebastian Fricke:
> On 11.02.2021 15:42, Heiko Stübner wrote:
> >Am Donnerstag, 11. Februar 2021, 06:25:15 CET schrieb Sebastian Fricke:
> >> On 10.02.2021 12:15, Heiko Stübner wrote:
> >> >Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner:
> >> >> I did some tests myself today as well and can confirm your
> >> >> hdmi related finding - at least when plugged in on boot.
> >> >>
> >> >> I tried some combinations of camera vs. hdmi and it seems
> >> >> really only when hdmi is plugged in on boot
> >> >
> >> >as you can see in v2, it should work now even with hdmi
> >> >connected on boot. My patch ignored the grf-clock when
> >> >doing the grf-based init.
> >> >
> >> >All clocks are on during boot and I guess the hdmi-driver
> >> >did disable it after its probe. The phy_power_on functions
> >> >did handle it correctly already, so it was only happening
> >> >with hdmi connected on boot.
> >>
> >> Thank you very much for solving that problem, I've tested the scenarios
> >> described below and it works like a charm. (With your V2)
> >> >
> >> >
> >> >Btw. do you plan on submitting your ov13850 driver
> >> >soonish?
> >>
> >> Actually, I have posted the patch already see here:
> >> https://patchwork.kernel.org/project/linux-media/patch/20210130182313.32903-2-sebastian.fricke@posteo.net/
> >
> >very cool to see
> >
> >> I currently review the requested changes and questions and will soon
> >> post a second version, but I expect quite some time until it is actually
> >> merged.
> >
> >could you Cc me on future versions?
> 
> Sure will do :)

by the way, you could also answer the v2 series with a

Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>

so we get some coverage :-)

Thanks
Heiko