[PATCH v3 00/25] drm/sun4i: Allwinner A64 MIPI-DSI support

[PATCH v3 00/25] drm/sun4i: Allwinner A64 MIPI-DSI support

[Jagan Teki]

This series fixed the issues related to work DSI on 2-lane panel
which is reported on previous version[1].

Few comments from previous version still in discussion, but I just
send this version just to group all working changes together.
anyway I will fix in this in next version if any.

PLL_MIPI min rate is still weird, I tried many possible dclock rate
from panel driver to satisfy manual suggested min rate 500MHz but
none working so eventually moved 300MHz. any inputs on this area
are welcome.

All these changes are tested in 2-lane, 4-lane MIPI-DSI panels.

If anyone wants to test, use this repo [2] with WIP-A64-DSI branch.

Any inputs,
Jagan.

[2] https://github.com/amarula/linux-amarula
[1] https://patchwork.kernel.org/cover/10653275/

Jagan Teki (25):
  clk: sunxi-ng: a64: Fix gate bit of DSI DPHY
  clk: sunxi-ng: Add check for minimal rate to NKM PLLs
  clk: sunxi-ng: Add check for maximum rate to NKM PLLs
  drm/sun4i: sun6i_mipi_dsi: Add Allwinner A64 MIPI DSI support
  dt-bindings: sun6i-dsi: Add compatible for A64 MIPI DSI
  drm/sun4i: sun6i_mipi_dsi: Add DSI Generic short write 2 param
    transfer
  drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation
  drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits
  drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay
  drm/sun4i: sun6i_mipi_dsi: Fix DSI hbp timing value
  drm/sun4i: sun6i_mipi_dsi: Fix DSI hblk timing calculation
  drm/sun4i: sun6i_mipi_dsi: Add DSI hblk packet overhead
  drm/sun4i: sun6i_mipi_dsi: Fix DSI hfp timing value
  drm/sun4i: sun6i_mipi_dsi: Increase hfp packet overhead
  drm/sun4i: sun6i_mipi_dsi: Set proper vblk timing calculation
  drm/sun4i: sun6i_mipi_dsi: Add support for VCC-DSI voltage regulator
  dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB
    bridge
  drm/panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge
  dt-bindings: panel: Add Techstar TS8550B MIPI DSI panel
  drm/panel: Add Techstar TS8550B MIPI-DSI LCD panel
  clk: sunxi-ng: a64: Add min and max rate for PLL_MIPI
  dt-bindings: sun6i-dsi: Add compatible for A64 DPHY
  arm64: dts: allwinner: a64: Add DSI pipeline
  [DO NOT MERGE] arm64: dts: allwinner: bananapi-m64: Bananapi S070WV20-CT16 DSI panel
  arm64: dts: allwinner: a64-amarula-relic: Enable Techstar TS8550B
    MIPI-DSI panel

 .../display/panel/bananapi,s070wv20-ct16.txt  |  31 +-
 .../display/panel/techstar,ts8550b.txt        |  20 +
 .../bindings/display/sunxi/sun6i-dsi.txt      |   3 +-
 .../allwinner/sun50i-a64-amarula-relic.dts    |  46 +++
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts |  42 +++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  45 +++
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c         |   4 +-
 drivers/clk/sunxi-ng/ccu_nkm.c                |  14 +
 drivers/clk/sunxi-ng/ccu_nkm.h                |   2 +
 drivers/gpu/drm/panel/Kconfig                 |  18 +
 drivers/gpu/drm/panel/Makefile                |   2 +
 .../panel/panel-bananapi-s070wv20-icn6211.c   | 336 +++++++++++++++++
 .../gpu/drm/panel/panel-techstar-ts8550b.c    | 346 ++++++++++++++++++
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c        | 122 ++++--
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h        |   8 +
 15 files changed, 1009 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/techstar,ts8550b.txt
 create mode 100644 drivers/gpu/drm/panel/panel-bananapi-s070wv20-icn6211.c
 create mode 100644 drivers/gpu/drm/panel/panel-techstar-ts8550b.c

-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 01/25] clk: sunxi-ng: a64: Fix gate bit of DSI DPHY

[Jagan Teki]

DSI DPHY gate bit on MIPI DSI clock register is bit 15
not bit 30.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Acked-by: Stephen Boyd <sboyd@kernel.org>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- collect Stephen Ack
- add tested credit
Changes for v2:
- none

 drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index f7d297368eb2..019d67bf97c4 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -581,7 +581,7 @@ static const char * const dsi_dphy_parents[] = { "pll-video0", "pll-periph0" };
 static const u8 dsi_dphy_table[] = { 0, 2, };
 static SUNXI_CCU_M_WITH_MUX_TABLE_GATE(dsi_dphy_clk, "dsi-dphy",
 				       dsi_dphy_parents, dsi_dphy_table,
-				       0x168, 0, 4, 8, 2, BIT(31), CLK_SET_RATE_PARENT);
+				       0x168, 0, 4, 8, 2, BIT(15), CLK_SET_RATE_PARENT);
 
 static SUNXI_CCU_M_WITH_GATE(gpu_clk, "gpu", "pll-gpu",
 			     0x1a0, 0, 3, BIT(31), CLK_SET_RATE_PARENT);
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 02/25] clk: sunxi-ng: Add check for minimal rate to NKM PLLs

[Jagan Teki]

Some NKM PLLs doesn't work well when their output clock rate is set below
certain rate.

So, add support for minimal rate for relevant PLLs.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Acked-by: Stephen Boyd <sboyd@kernel.org>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- collect Stephen Ack
- add tested credit
Changes for v2:
- none

 drivers/clk/sunxi-ng/ccu_nkm.c | 7 +++++++
 drivers/clk/sunxi-ng/ccu_nkm.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/clk/sunxi-ng/ccu_nkm.c b/drivers/clk/sunxi-ng/ccu_nkm.c
index 841840e35e61..d17539dc88dd 100644
--- a/drivers/clk/sunxi-ng/ccu_nkm.c
+++ b/drivers/clk/sunxi-ng/ccu_nkm.c
@@ -125,6 +125,13 @@ static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux,
 	if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
 		rate *= nkm->fixed_post_div;
 
+	if (rate < nkm->min_rate) {
+		rate = nkm->min_rate;
+		if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
+			rate /= nkm->fixed_post_div;
+		return rate;
+	}
+
 	ccu_nkm_find_best(*parent_rate, rate, &_nkm);
 
 	rate = *parent_rate * _nkm.n * _nkm.k / _nkm.m;
diff --git a/drivers/clk/sunxi-ng/ccu_nkm.h b/drivers/clk/sunxi-ng/ccu_nkm.h
index cc6efb70a102..ff5bd00f429f 100644
--- a/drivers/clk/sunxi-ng/ccu_nkm.h
+++ b/drivers/clk/sunxi-ng/ccu_nkm.h
@@ -35,6 +35,7 @@ struct ccu_nkm {
 	struct ccu_mux_internal	mux;
 
 	unsigned int		fixed_post_div;
+	unsigned int		min_rate;
 
 	struct ccu_common	common;
 };
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 03/25] clk: sunxi-ng: Add check for maximum rate to NKM PLLs

[Jagan Teki]

Some NKM PLLs, frequency can be set above PLL working range.

Add a constraint for maximum supported rate. This way, drivers can
specify which is maximum allowed rate for PLL.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Acked-by: Stephen Boyd <sboyd@kernel.org>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- collect Stephen Ack
- add tested credit
Changes for v2:
- none

 drivers/clk/sunxi-ng/ccu_nkm.c | 7 +++++++
 drivers/clk/sunxi-ng/ccu_nkm.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/clk/sunxi-ng/ccu_nkm.c b/drivers/clk/sunxi-ng/ccu_nkm.c
index d17539dc88dd..574fd2cd2a79 100644
--- a/drivers/clk/sunxi-ng/ccu_nkm.c
+++ b/drivers/clk/sunxi-ng/ccu_nkm.c
@@ -132,6 +132,13 @@ static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux,
 		return rate;
 	}
 
+	if (nkm->max_rate && rate > nkm->max_rate) {
+		rate = nkm->max_rate;
+		if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
+			rate /= nkm->fixed_post_div;
+		return rate;
+	}
+
 	ccu_nkm_find_best(*parent_rate, rate, &_nkm);
 
 	rate = *parent_rate * _nkm.n * _nkm.k / _nkm.m;
diff --git a/drivers/clk/sunxi-ng/ccu_nkm.h b/drivers/clk/sunxi-ng/ccu_nkm.h
index ff5bd00f429f..c82590481188 100644
--- a/drivers/clk/sunxi-ng/ccu_nkm.h
+++ b/drivers/clk/sunxi-ng/ccu_nkm.h
@@ -36,6 +36,7 @@ struct ccu_nkm {
 
 	unsigned int		fixed_post_div;
 	unsigned int		min_rate;
+	unsigned int		max_rate;
 
 	struct ccu_common	common;
 };
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 04/25] drm/sun4i: sun6i_mipi_dsi: Add Allwinner A64 MIPI DSI support

[Jagan Teki]

The MIPI DSI controller on Allwinner A64 is similar to
Allwinner A31 without support of DSI mod clock(CLK_DSI_SCLK)

So, alter has_mod_clk bool via driver data for respective
SoC's compatible.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- add tested credit
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 47 ++++++++++++++++++++------
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  5 +++
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index e3b34a345546..8e9c76febca2 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -10,6 +10,7 @@
 #include <linux/component.h>
 #include <linux/crc-ccitt.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
@@ -981,6 +982,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 	dsi->host.ops = &sun6i_dsi_host_ops;
 	dsi->host.dev = dev;
 
+	dsi->variant = of_device_get_match_data(dev);
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(base)) {
@@ -1001,17 +1004,20 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 		return PTR_ERR(dsi->reset);
 	}
 
-	dsi->mod_clk = devm_clk_get(dev, "mod");
-	if (IS_ERR(dsi->mod_clk)) {
-		dev_err(dev, "Couldn't get the DSI mod clock\n");
-		return PTR_ERR(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk) {
+		dsi->mod_clk = devm_clk_get(dev, "mod");
+		if (IS_ERR(dsi->mod_clk)) {
+			dev_err(dev, "Couldn't get the DSI mod clock\n");
+			return PTR_ERR(dsi->mod_clk);
+		}
 	}
 
 	/*
 	 * In order to operate properly, that clock seems to be always
 	 * set to 297MHz.
 	 */
-	clk_set_rate_exclusive(dsi->mod_clk, 297000000);
+	if (dsi->variant->has_mod_clk)
+		clk_set_rate_exclusive(dsi->mod_clk, 297000000);
 
 	dphy_node = of_parse_phandle(dev->of_node, "phys", 0);
 	ret = sun6i_dphy_probe(dsi, dphy_node);
@@ -1043,7 +1049,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 	pm_runtime_disable(dev);
 	sun6i_dphy_remove(dsi);
 err_unprotect_clk:
-	clk_rate_exclusive_put(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_rate_exclusive_put(dsi->mod_clk);
 	return ret;
 }
 
@@ -1056,7 +1063,8 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
 	mipi_dsi_host_unregister(&dsi->host);
 	pm_runtime_disable(dev);
 	sun6i_dphy_remove(dsi);
-	clk_rate_exclusive_put(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_rate_exclusive_put(dsi->mod_clk);
 
 	return 0;
 }
@@ -1066,7 +1074,8 @@ static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev)
 	struct sun6i_dsi *dsi = dev_get_drvdata(dev);
 
 	reset_control_deassert(dsi->reset);
-	clk_prepare_enable(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_prepare_enable(dsi->mod_clk);
 
 	/*
 	 * Enable the DSI block.
@@ -1094,7 +1103,8 @@ static int __maybe_unused sun6i_dsi_runtime_suspend(struct device *dev)
 {
 	struct sun6i_dsi *dsi = dev_get_drvdata(dev);
 
-	clk_disable_unprepare(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_disable_unprepare(dsi->mod_clk);
 	reset_control_assert(dsi->reset);
 
 	return 0;
@@ -1106,9 +1116,24 @@ static const struct dev_pm_ops sun6i_dsi_pm_ops = {
 			   NULL)
 };
 
+static const struct sun6i_dsi_variant sun6i_a31_dsi = {
+	.has_mod_clk = true,
+};
+
+static const struct sun6i_dsi_variant sun50i_a64_dsi = {
+	.has_mod_clk = false,
+};
+
 static const struct of_device_id sun6i_dsi_of_table[] = {
-	{ .compatible = "allwinner,sun6i-a31-mipi-dsi" },
-	{ }
+	{
+		.compatible = "allwinner,sun6i-a31-mipi-dsi",
+		.data = &sun6i_a31_dsi,
+	},
+	{
+		.compatible = "allwinner,sun50i-a64-mipi-dsi",
+		.data = &sun50i_a64_dsi,
+	},
+	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);
 
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index dbbc5b3ecbda..597b62227019 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -20,6 +20,10 @@ struct sun6i_dphy {
 	struct reset_control	*reset;
 };
 
+struct sun6i_dsi_variant {
+	bool			has_mod_clk;
+};
+
 struct sun6i_dsi {
 	struct drm_connector	connector;
 	struct drm_encoder	encoder;
@@ -35,6 +39,7 @@ struct sun6i_dsi {
 	struct sun4i_drv	*drv;
 	struct mipi_dsi_device	*device;
 	struct drm_panel	*panel;
+	const struct sun6i_dsi_variant	*variant;
 };
 
 static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 05/25] dt-bindings: sun6i-dsi: Add compatible for A64 MIPI DSI

[Jagan Teki]

The MIPI DSI controller on Allwinner A64 is similar to
Allwinner A31 without support of DSI mod clock.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- add tested credit
Changes for v2:
- none

 Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt
index 6a6cf5de08b0..9fa6e7a758ad 100644
--- a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt
+++ b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt
@@ -12,6 +12,7 @@ The DSI Encoder generates the DSI signal from the TCON's.
 Required properties:
   - compatible: value must be one of:
     * allwinner,sun6i-a31-mipi-dsi
+    * allwinner,sun50i-a64-mipi-dsi
   - reg: base address and size of memory-mapped region
   - interrupts: interrupt associated to this IP
   - clocks: phandles to the clocks feeding the DSI encoder
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 06/25] drm/sun4i: sun6i_mipi_dsi: Add DSI Generic short write 2 param transfer

[Jagan Teki]

Short transfer write support for DCS and Generic transfer types
share similar way to process command sequence in DSI block so
add generic write 2 param transfer type macro so-that the panels
which are requesting similar transfer type may process properly.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- add tested credit
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 8e9c76febca2..fc8560607147 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -871,6 +871,7 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
 	switch (msg->type) {
 	case MIPI_DSI_DCS_SHORT_WRITE:
 	case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
+	case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
 		ret = sun6i_dsi_dcs_write_short(dsi, msg);
 		break;
 
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v3 06/25] drm/sun4i: sun6i_mipi_dsi: Add DSI Generic short write 2 param transfer

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

On Fri, Oct 26, 2018 at 08:13:25PM +0530, Jagan Teki wrote:
> Short transfer write support for DCS and Generic transfer types
> share similar way to process command sequence in DSI block so
> add generic write 2 param transfer type macro so-that the panels
> which are requesting similar transfer type may process properly.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Jagan Teki <jagan@amarulasolutions.com>

I'm not sure why you have a Tested-by tag here. Of course we're
expecting you to test your patches before sending them.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v3 07/25] drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation

[Jagan Teki]

The horizontal and vertical back porch calculation in BSP
code is simply following the Linux drm comment diagram, in
include/drm/drm_modes.h which is

[hv]back porch = [hv]total - [hv]sync_end

BSP code form BPI-M64-bsp is calculating vertical back porch as
(from linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c)

timmings->ver_sync_time= panel_info->lcd_vspw;
timmings->ver_back_porch= panel_info->lcd_vbp-panel_info->lcd_vspw;

vbp = panel->lcd_vbp;
vspw = panel->lcd_vspw;
dsi_dev[sel]->dsi_basic_size0.bits.vbp = vbp-vspw;
dsi_dev[sel]->dsi_basic_size0.bits.vbp = panel->lcd_vbp - panel->lcd_vspw;
=>  timmings->ver_back_porch + panel_info->lcd_vspw - panel_info->lcd_vspw
=>  timmings->ver_back_porch
=>  mode->vtotal - mode->end

Which evatually same as mode->vtotal - mode->vsync_end so update the
same in SUN6I_DSI_BASIC_SIZE0_VBP

On the information note, existing SUN6I_DSI_BASIC_SIZE0_VSA is proper
value.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- Fixed proper commit message
- add tested credit
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index fc8560607147..3a1d48bc1996 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -526,8 +526,8 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	regmap_write(dsi->regs, SUN6I_DSI_BASIC_SIZE0_REG,
 		     SUN6I_DSI_BASIC_SIZE0_VSA(mode->vsync_end -
 					       mode->vsync_start) |
-		     SUN6I_DSI_BASIC_SIZE0_VBP(mode->vsync_start -
-					       mode->vdisplay));
+		     SUN6I_DSI_BASIC_SIZE0_VBP(mode->vtotal -
+					       mode->vsync_end));
 
 	regmap_write(dsi->regs, SUN6I_DSI_BASIC_SIZE1_REG,
 		     SUN6I_DSI_BASIC_SIZE1_VACT(mode->vdisplay) |
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 08/25] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits

[Jagan Teki]

TCON DRQ set bits for non-burst DSI mode can computed via
horizontal front porch instead of front porch + sync timings.

BSP code form BPI-M64-bsp is computing TCON DRQ set bits
for non-burts as
(in linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)

=> panel->lcd_ht -    panel->lcd_x - panel->lcd_hbp
=> (timmings->hor_front_porch + panel->lcd_hbp + panel->lcd_x)
   - panel->lcd_x - panel->hbp
=> timmings->hor_front_porch
=> mode->hsync_start - mode->hdisplay

So, update the DRQ set bits accordingly.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- Fixed proper commit message
- add tested credit
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 3a1d48bc1996..8d154cf2e6d6 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -367,9 +367,9 @@ static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
 	struct mipi_dsi_device *device = dsi->device;
 	u32 val = 0;
 
-	if ((mode->hsync_end - mode->hdisplay) > 20) {
+	if ((mode->hsync_start - mode->hdisplay) > 20) {
 		/* Maaaaaagic */
-		u16 drq = (mode->hsync_end - mode->hdisplay) - 20;
+		u16 drq = (mode->hsync_start - mode->hdisplay) - 20;
 
 		drq *= mipi_dsi_pixel_format_to_bpp(device->format);
 		drq /= 32;
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 09/25] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay

[Jagan Teki]

Video start delay can be computed by subtracting total vertical
timing with front porch timing and with adding 1 delay line for TCON.

BSP code form BPI-M64-bsp is computing video start delay as
(in linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)

u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp;
=> (panel->lcd_vt) - panel->lcd_y - (panel->lcd_vbp)
=> (timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y)
   - panel->lcd_y - (panel->lcd_vbp)
=> timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y
  			     - panel->lcd_y - panel->lcd_vbp
=> timmings->ver_front_porch

So, update the start delay computation accordingly.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- Fixed proper commit message
- add tested credit
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 8d154cf2e6d6..6bece492b1f7 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -358,7 +358,17 @@ static void sun6i_dsi_inst_init(struct sun6i_dsi *dsi,
 static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi,
 					   struct drm_display_mode *mode)
 {
-	return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1;
+	u32 vfp = mode->vsync_start - mode->vdisplay;
+	u32 start_delay;
+
+	start_delay = mode->vtotal - vfp + 1;
+	if (start_delay > mode->vtotal)
+		start_delay -= mode->vtotal;
+
+	if (!start_delay)
+		start_delay = 1;
+
+	return start_delay;
 }
 
 static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 10/25] drm/sun4i: sun6i_mipi_dsi: Fix DSI hbp timing value

[Jagan Teki]

Current driver is calculating hbp maximum value by subtracting
hsync_start with hdisplay which is front porch value, but the
hbp refers to back porch.

Back porch value is calculating by subtracting htotal with
hsync_end as per drm_mode timings, and BSP code from BPI-M64-bsp
is eventually following the same.

BPI-M64-bsp is computing hbp as
(in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
dsi_hbp = (hbp-hspw)*dsi_pixel_bits[format]/8 - (4+2);
=> (panel->lcd_hbp - timmings->hor_sync_time)
=> (timmings->hor_back_porch + timmings->hor_sync_time - timmings->hor_sync_time)
=> timmings->hor_back_porch
=> mode->htotal - mode->hsync_end

So, update the MIPI-DSI hbp value accordingly.

Tested on 2-lane, 4-lane DSI LCD panels.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- Fixed proper commit message
- add tested credit
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 6bece492b1f7..78bad71045ca 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -482,7 +482,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	 */
 #define HBP_PACKET_OVERHEAD	6
 	hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
-		  (mode->hsync_start - mode->hdisplay) * Bpp - HBP_PACKET_OVERHEAD);
+		  (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
 
 	/*
 	 * The frontporch is set using a blanking packet (4 bytes +
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 11/25] drm/sun4i: sun6i_mipi_dsi: Fix DSI hblk timing calculation

[Jagan Teki]

hblk is adding line with all porch timing values, or timings
values from htotal without sync time.

Current driver is subtracting htotal with hsa, but the hsa
is bounded with packet overhead. For real hblk calculation
needed by subtracting htotal with back and front porch values
and BSP code BPI-M64-bsp is eventually following the same.

BPI-M64-bsp is computing hbp as
(in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2);
=> (timmings->hor_total_time - timmings->hor_sync_time)
=> (mode->htotal - (mode->hsync_end - mode->hsync_start))

So, update the DSI hblk timing accordingly.

Tested on 2-lane, 4-lane MIPI-DSI LCD panels.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- new patch
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 78bad71045ca..596e560263bf 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -495,7 +495,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	/*
 	 * hblk seems to be the line + porches length.
 	 */
-	hblk = mode->htotal * Bpp - hsa;
+	hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp;
 
 	/*
 	 * And I'm not entirely sure what vblk is about. The driver in
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 12/25] drm/sun4i: sun6i_mipi_dsi: Add DSI hblk packet overhead

[Jagan Teki]

Add 10 bytes packet overhead for hblk where blank is set using
a blanking packet like (4 bytes + 4 bytes + payload + 2 bytes)

This is according to BSP code from BPI-M64-bsp
(in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2);

So, add 10 bytes packet overhead for DSI hblk.

Tested on 2-lane, 4-lane MIPI-DSI LCD panels.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- new patch
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 596e560263bf..cf42be1f1ba1 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 {
 	struct mipi_dsi_device *device = dsi->device;
 	unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
-	u16 hbp, hfp, hsa, hblk, vblk;
+	u16 hbp, hfp, hsa, hblk_max, hblk, vblk;
 	size_t bytes;
 	u8 *buffer;
 
@@ -494,8 +494,13 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 
 	/*
 	 * hblk seems to be the line + porches length.
+	 * The blank is set using a blanking packet (4 bytes + 4 bytes +
+	 * payload + 2 bytes). So minimal size is 10 bytes
 	 */
-	hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp;
+#define HBLK_PACKET_OVERHEAD	10
+	hblk_max = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp;
+	hblk_max -= HBLK_PACKET_OVERHEAD;
+	hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max);
 
 	/*
 	 * And I'm not entirely sure what vblk is about. The driver in
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v3 12/25] drm/sun4i: sun6i_mipi_dsi: Add DSI hblk packet overhead

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]

On Fri, Oct 26, 2018 at 08:13:31PM +0530, Jagan Teki wrote:
> Add 10 bytes packet overhead for hblk where blank is set using
> a blanking packet like (4 bytes + 4 bytes + payload + 2 bytes)
> 
> This is according to BSP code from BPI-M64-bsp
> (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
> dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2);
> 
> So, add 10 bytes packet overhead for DSI hblk.
> 
> Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - new patch
> Changes for v2:
> - none
> 
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 596e560263bf..cf42be1f1ba1 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
>  {
>  	struct mipi_dsi_device *device = dsi->device;
>  	unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> -	u16 hbp, hfp, hsa, hblk, vblk;
> +	u16 hbp, hfp, hsa, hblk_max, hblk, vblk;
>  	size_t bytes;
>  	u8 *buffer;
>  
> @@ -494,8 +494,13 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
>  
>  	/*
>  	 * hblk seems to be the line + porches length.
> +	 * The blank is set using a blanking packet (4 bytes + 4 bytes +
> +	 * payload + 2 bytes). So minimal size is 10 bytes
>  	 */
> -	hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp;
> +#define HBLK_PACKET_OVERHEAD	10
> +	hblk_max = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp;
> +	hblk_max -= HBLK_PACKET_OVERHEAD;
> +	hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max);

I'd rather use the same convention than the other assignments done
before in that function.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 12/25] drm/sun4i: sun6i_mipi_dsi: Add DSI hblk packet overhead

[Jagan Teki]

On Mon, Oct 29, 2018 at 2:52 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Fri, Oct 26, 2018 at 08:13:31PM +0530, Jagan Teki wrote:
> > Add 10 bytes packet overhead for hblk where blank is set using
> > a blanking packet like (4 bytes + 4 bytes + payload + 2 bytes)
> >
> > This is according to BSP code from BPI-M64-bsp
> > (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
> > dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2);
> >
> > So, add 10 bytes packet overhead for DSI hblk.
> >
> > Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v3:
> > - new patch
> > Changes for v2:
> > - none
> >
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 596e560263bf..cf42be1f1ba1 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> >  {
> >       struct mipi_dsi_device *device = dsi->device;
> >       unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> > -     u16 hbp, hfp, hsa, hblk, vblk;
> > +     u16 hbp, hfp, hsa, hblk_max, hblk, vblk;
> >       size_t bytes;
> >       u8 *buffer;
> >
> > @@ -494,8 +494,13 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> >
> >       /*
> >        * hblk seems to be the line + porches length.
> > +      * The blank is set using a blanking packet (4 bytes + 4 bytes +
> > +      * payload + 2 bytes). So minimal size is 10 bytes
> >        */
> > -     hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp;
> > +#define HBLK_PACKET_OVERHEAD 10
> > +     hblk_max = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp;
> > +     hblk_max -= HBLK_PACKET_OVERHEAD;
> > +     hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max);
>
> I'd rather use the same convention than the other assignments done
> before in that function.

Yes, old code did use hsa. but it's added 10 bytes packet head instead
of subtracting it.

hblk = mode->htotal * Bpp - hsa;
=> mode->htotal * Bpp - (mode->hsync_end - mode->hsync_start) * Bpp -
HSA_PACKET_OVERHEAD);
=> (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp +
HSA_PACKET_OVERHEAD;

And it should be
(mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp -
HSA_PACKET_OVERHEAD;

This patch is simply doing the same by explicitly adding packet over
ahead macro, which again used in hfp.

Re: [PATCH v3 12/25] drm/sun4i: sun6i_mipi_dsi: Add DSI hblk packet overhead

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3353 bytes --]

On Mon, Oct 29, 2018 at 07:56:27PM +0530, Jagan Teki wrote:
> On Mon, Oct 29, 2018 at 2:52 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Fri, Oct 26, 2018 at 08:13:31PM +0530, Jagan Teki wrote:
> > > Add 10 bytes packet overhead for hblk where blank is set using
> > > a blanking packet like (4 bytes + 4 bytes + payload + 2 bytes)
> > >
> > > This is according to BSP code from BPI-M64-bsp
> > > (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
> > > dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2);
> > >
> > > So, add 10 bytes packet overhead for DSI hblk.
> > >
> > > Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > > Changes for v3:
> > > - new patch
> > > Changes for v2:
> > > - none
> > >
> > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > index 596e560263bf..cf42be1f1ba1 100644
> > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > @@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> > >  {
> > >       struct mipi_dsi_device *device = dsi->device;
> > >       unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> > > -     u16 hbp, hfp, hsa, hblk, vblk;
> > > +     u16 hbp, hfp, hsa, hblk_max, hblk, vblk;
> > >       size_t bytes;
> > >       u8 *buffer;
> > >
> > > @@ -494,8 +494,13 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> > >
> > >       /*
> > >        * hblk seems to be the line + porches length.
> > > +      * The blank is set using a blanking packet (4 bytes + 4 bytes +
> > > +      * payload + 2 bytes). So minimal size is 10 bytes
> > >        */
> > > -     hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp;
> > > +#define HBLK_PACKET_OVERHEAD 10
> > > +     hblk_max = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp;
> > > +     hblk_max -= HBLK_PACKET_OVERHEAD;
> > > +     hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max);
> >
> > I'd rather use the same convention than the other assignments done
> > before in that function.
> 
> Yes, old code did use hsa. but it's added 10 bytes packet head instead
> of subtracting it.
> 
> hblk = mode->htotal * Bpp - hsa;
> => mode->htotal * Bpp - (mode->hsync_end - mode->hsync_start) * Bpp -
> HSA_PACKET_OVERHEAD);
> => (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp +
> HSA_PACKET_OVERHEAD;
> 
> And it should be
> (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp -
> HSA_PACKET_OVERHEAD;
> 
> This patch is simply doing the same by explicitly adding packet over
> ahead macro, which again used in hfp.

That's not my point.

The rest of the driver uses a construct that would be:

hblk = max((unsigned int)HBLK_PACKET_OVERHEAD,
           (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp - HBLK_PACKET_OVERHEAD);

We want to remain consistent.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v3 13/25] drm/sun4i: sun6i_mipi_dsi: Fix DSI hfp timing value

[Jagan Teki]

Current driver is calculating hfp maximum value by subtracting
htotal with hsync_end which is front back value, but the
hpp refers to front porch.

Front porch value is calculating by subtracting hsync_start with
hdisplay as per drm_mode timings, and BSP code from BPI-M64-bsp
is eventually following the same.

BPI-M64-bsp is computing hfp as
(in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
dsi_hbp = (hbp-hspw)*dsi_pixel_bits[format]/8 - (4+2);
dsi_hact = x * dsi_pixel_bits[format]/8;
dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2);
dsi_hfp = dsi_hblk - (4+dsi_hact+2) - (4+dsi_hbp+2);

Example,
u32 fmt = dsi_pixel_bits[format]/8;
=> ((ht-hspw)*fmt - 10) - (6 + x * fmt) - (6 + (hbp-hspw)*fmt - 6)
=> (ht - hspw - x - (hbp - hspw)) * fmt - 16
=> (ht - x - hbp) * fmt - 16
=> (ht - x - (timmings->hor_total_time - timmings->hor_front_porch - x)
* fmt - 16
=> (timmings->hor_total_time - x - timmings->hor_total_time +
timmings->hor_front_porch + x) * fmt - 16
=> timmings->hor_front_porch * fmt - 16

So, update the DSI hfp timing accordingly.

Tested on 2-lane, 4-lane MIPI-DSI LCD panels.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- new patch
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index cf42be1f1ba1..6584b51736a9 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -490,7 +490,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	 */
 #define HFP_PACKET_OVERHEAD	6
 	hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
-		  (mode->htotal - mode->hsync_end) * Bpp - HFP_PACKET_OVERHEAD);
+		  (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
 
 	/*
 	 * hblk seems to be the line + porches length.
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 14/25] drm/sun4i: sun6i_mipi_dsi: Increase hfp packet overhead

[Jagan Teki]

Increase the hfp packet overhead with another 10 bytes, the extra
10 bytes(which is hblk packet overhead) is adding for hfp packet
overhead since hfp depends on hblk.

This is truely as per BSP code from BPI-M64-bsp.
The real computation from BSP is
(in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
dsi_hbp = (hbp-hspw)*dsi_pixel_bits[format]/8 - (4+2);
dsi_hact = x * dsi_pixel_bits[format]/8;
dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2);
dsi_hfp = dsi_hblk - (4+dsi_hact+2) - (4+dsi_hbp+2);

Example,
u32 fmt = dsi_pixel_bits[format]/8;
=> ((ht-hspw)*fmt - 10) - (6 + x * fmt) - (6 + (hbp-hspw)*fmt - 6)
=> (ht - hspw - x - (hbp - hspw)) * fmt - 16
=> (ht - x - hbp) * fmt - 16
=> (ht - x - (timmings->hor_total_time - timmings->hor_front_porch - x)
* fmt - 16
=> (timmings->hor_total_time - x - timmings->hor_total_time +
timmings->hor_front_porch + x) * fmt - 16
=> timmings->hor_front_porch * fmt - 16

So, increase the DSI hfp packet overhead by hblk packet overhead.

Tested on 2-lane, 4-lane MIPI-DSI LCD panels.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- new patch
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 6584b51736a9..20e330186b7f 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 {
 	struct mipi_dsi_device *device = dsi->device;
 	unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
-	u16 hbp, hfp, hsa, hblk_max, hblk, vblk;
+	u16 hbp, hfp_pkt_overhead, hfp, hsa, hblk_max, hblk, vblk;
 	size_t bytes;
 	u8 *buffer;
 
@@ -484,14 +484,6 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
 		  (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
 
-	/*
-	 * The frontporch is set using a blanking packet (4 bytes +
-	 * payload + 2 bytes). Its minimal size is therefore 6 bytes
-	 */
-#define HFP_PACKET_OVERHEAD	6
-	hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
-		  (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
-
 	/*
 	 * hblk seems to be the line + porches length.
 	 * The blank is set using a blanking packet (4 bytes + 4 bytes +
@@ -502,6 +494,18 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	hblk_max -= HBLK_PACKET_OVERHEAD;
 	hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max);
 
+	/*
+	 * The frontporch is set using a blanking packet (4 bytes +
+	 * payload + 2 bytes). Its minimal size is therefore 6 bytes
+	 *
+	 * According to BSP code, extra 10 bytes(which is hblk packet overhead)
+	 * is adding for hfp packet overhead since hfp depends on hblk.
+	 */
+#define HFP_PACKET_OVERHEAD	6
+	hfp_pkt_overhead = (HFP_PACKET_OVERHEAD + HBLK_PACKET_OVERHEAD);
+	hfp = max((unsigned int)hfp_pkt_overhead,
+		  (mode->hsync_start - mode->hdisplay) * Bpp - hfp_pkt_overhead);
+
 	/*
 	 * And I'm not entirely sure what vblk is about. The driver in
 	 * Allwinner BSP is using a rather convoluted calculation
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v3 14/25] drm/sun4i: sun6i_mipi_dsi: Increase hfp packet overhead

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3571 bytes --]

On Fri, Oct 26, 2018 at 08:13:33PM +0530, Jagan Teki wrote:
> Increase the hfp packet overhead with another 10 bytes, the extra
> 10 bytes(which is hblk packet overhead) is adding for hfp packet
> overhead since hfp depends on hblk.
> 
> This is truely as per BSP code from BPI-M64-bsp.
> The real computation from BSP is
> (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
> dsi_hbp = (hbp-hspw)*dsi_pixel_bits[format]/8 - (4+2);
> dsi_hact = x * dsi_pixel_bits[format]/8;
> dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2);
> dsi_hfp = dsi_hblk - (4+dsi_hact+2) - (4+dsi_hbp+2);
> 
> Example,
> u32 fmt = dsi_pixel_bits[format]/8;
> => ((ht-hspw)*fmt - 10) - (6 + x * fmt) - (6 + (hbp-hspw)*fmt - 6)
> => (ht - hspw - x - (hbp - hspw)) * fmt - 16
> => (ht - x - hbp) * fmt - 16
> => (ht - x - (timmings->hor_total_time - timmings->hor_front_porch - x)
> * fmt - 16
> => (timmings->hor_total_time - x - timmings->hor_total_time +
> timmings->hor_front_porch + x) * fmt - 16
> => timmings->hor_front_porch * fmt - 16
> 
> So, increase the DSI hfp packet overhead by hblk packet overhead.
> 
> Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - new patch
> Changes for v2:
> - none
> 
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 6584b51736a9..20e330186b7f 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
>  {
>  	struct mipi_dsi_device *device = dsi->device;
>  	unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> -	u16 hbp, hfp, hsa, hblk_max, hblk, vblk;
> +	u16 hbp, hfp_pkt_overhead, hfp, hsa, hblk_max, hblk, vblk;
>  	size_t bytes;
>  	u8 *buffer;
>  
> @@ -484,14 +484,6 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
>  	hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
>  		  (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
>  
> -	/*
> -	 * The frontporch is set using a blanking packet (4 bytes +
> -	 * payload + 2 bytes). Its minimal size is therefore 6 bytes
> -	 */
> -#define HFP_PACKET_OVERHEAD	6
> -	hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
> -		  (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
> -
>  	/*
>  	 * hblk seems to be the line + porches length.
>  	 * The blank is set using a blanking packet (4 bytes + 4 bytes +
> @@ -502,6 +494,18 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
>  	hblk_max -= HBLK_PACKET_OVERHEAD;
>  	hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max);
>  
> +	/*
> +	 * The frontporch is set using a blanking packet (4 bytes +
> +	 * payload + 2 bytes). Its minimal size is therefore 6 bytes
> +	 *
> +	 * According to BSP code, extra 10 bytes(which is hblk packet overhead)
> +	 * is adding for hfp packet overhead since hfp depends on hblk.

"According to the BSP code, another 10 bytes (the HBLK packet
overhead) need to be added to the front porch overhead."

How has this been verified? Since we have registers to set all the
parameters, this seems kind of weird.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 14/25] drm/sun4i: sun6i_mipi_dsi: Increase hfp packet overhead

[Jagan Teki]

On Mon, Oct 29, 2018 at 2:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Fri, Oct 26, 2018 at 08:13:33PM +0530, Jagan Teki wrote:
> > Increase the hfp packet overhead with another 10 bytes, the extra
> > 10 bytes(which is hblk packet overhead) is adding for hfp packet
> > overhead since hfp depends on hblk.
> >
> > This is truely as per BSP code from BPI-M64-bsp.
> > The real computation from BSP is
> > (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
> > dsi_hbp = (hbp-hspw)*dsi_pixel_bits[format]/8 - (4+2);
> > dsi_hact = x * dsi_pixel_bits[format]/8;
> > dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2);
> > dsi_hfp = dsi_hblk - (4+dsi_hact+2) - (4+dsi_hbp+2);
> >
> > Example,
> > u32 fmt = dsi_pixel_bits[format]/8;
> > => ((ht-hspw)*fmt - 10) - (6 + x * fmt) - (6 + (hbp-hspw)*fmt - 6)
> > => (ht - hspw - x - (hbp - hspw)) * fmt - 16
> > => (ht - x - hbp) * fmt - 16
> > => (ht - x - (timmings->hor_total_time - timmings->hor_front_porch - x)
> > * fmt - 16
> > => (timmings->hor_total_time - x - timmings->hor_total_time +
> > timmings->hor_front_porch + x) * fmt - 16
> > => timmings->hor_front_porch * fmt - 16
> >
> > So, increase the DSI hfp packet overhead by hblk packet overhead.
> >
> > Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v3:
> > - new patch
> > Changes for v2:
> > - none
> >
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 22 +++++++++++++---------
> >  1 file changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 6584b51736a9..20e330186b7f 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> >  {
> >       struct mipi_dsi_device *device = dsi->device;
> >       unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> > -     u16 hbp, hfp, hsa, hblk_max, hblk, vblk;
> > +     u16 hbp, hfp_pkt_overhead, hfp, hsa, hblk_max, hblk, vblk;
> >       size_t bytes;
> >       u8 *buffer;
> >
> > @@ -484,14 +484,6 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> >       hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
> >                 (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
> >
> > -     /*
> > -      * The frontporch is set using a blanking packet (4 bytes +
> > -      * payload + 2 bytes). Its minimal size is therefore 6 bytes
> > -      */
> > -#define HFP_PACKET_OVERHEAD  6
> > -     hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
> > -               (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
> > -
> >       /*
> >        * hblk seems to be the line + porches length.
> >        * The blank is set using a blanking packet (4 bytes + 4 bytes +
> > @@ -502,6 +494,18 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> >       hblk_max -= HBLK_PACKET_OVERHEAD;
> >       hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max);
> >
> > +     /*
> > +      * The frontporch is set using a blanking packet (4 bytes +
> > +      * payload + 2 bytes). Its minimal size is therefore 6 bytes
> > +      *
> > +      * According to BSP code, extra 10 bytes(which is hblk packet overhead)
> > +      * is adding for hfp packet overhead since hfp depends on hblk.
>
> "According to the BSP code, another 10 bytes (the HBLK packet
> overhead) need to be added to the front porch overhead."
>
> How has this been verified? Since we have registers to set all the
> parameters, this seems kind of weird.

Verified based on the BSP calculation which I explained in commit
message and tested by validating these numbers with mainline vs bsp
code.

Re: [PATCH v3 14/25] drm/sun4i: sun6i_mipi_dsi: Increase hfp packet overhead

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4558 bytes --]

On Mon, Oct 29, 2018 at 07:57:56PM +0530, Jagan Teki wrote:
> On Mon, Oct 29, 2018 at 2:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Fri, Oct 26, 2018 at 08:13:33PM +0530, Jagan Teki wrote:
> > > Increase the hfp packet overhead with another 10 bytes, the extra
> > > 10 bytes(which is hblk packet overhead) is adding for hfp packet
> > > overhead since hfp depends on hblk.
> > >
> > > This is truely as per BSP code from BPI-M64-bsp.
> > > The real computation from BSP is
> > > (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
> > > dsi_hbp = (hbp-hspw)*dsi_pixel_bits[format]/8 - (4+2);
> > > dsi_hact = x * dsi_pixel_bits[format]/8;
> > > dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2);
> > > dsi_hfp = dsi_hblk - (4+dsi_hact+2) - (4+dsi_hbp+2);
> > >
> > > Example,
> > > u32 fmt = dsi_pixel_bits[format]/8;
> > > => ((ht-hspw)*fmt - 10) - (6 + x * fmt) - (6 + (hbp-hspw)*fmt - 6)
> > > => (ht - hspw - x - (hbp - hspw)) * fmt - 16
> > > => (ht - x - hbp) * fmt - 16
> > > => (ht - x - (timmings->hor_total_time - timmings->hor_front_porch - x)
> > > * fmt - 16
> > > => (timmings->hor_total_time - x - timmings->hor_total_time +
> > > timmings->hor_front_porch + x) * fmt - 16
> > > => timmings->hor_front_porch * fmt - 16
> > >
> > > So, increase the DSI hfp packet overhead by hblk packet overhead.
> > >
> > > Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > > Changes for v3:
> > > - new patch
> > > Changes for v2:
> > > - none
> > >
> > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 22 +++++++++++++---------
> > >  1 file changed, 13 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > index 6584b51736a9..20e330186b7f 100644
> > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > @@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> > >  {
> > >       struct mipi_dsi_device *device = dsi->device;
> > >       unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> > > -     u16 hbp, hfp, hsa, hblk_max, hblk, vblk;
> > > +     u16 hbp, hfp_pkt_overhead, hfp, hsa, hblk_max, hblk, vblk;
> > >       size_t bytes;
> > >       u8 *buffer;
> > >
> > > @@ -484,14 +484,6 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> > >       hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
> > >                 (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
> > >
> > > -     /*
> > > -      * The frontporch is set using a blanking packet (4 bytes +
> > > -      * payload + 2 bytes). Its minimal size is therefore 6 bytes
> > > -      */
> > > -#define HFP_PACKET_OVERHEAD  6
> > > -     hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
> > > -               (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
> > > -
> > >       /*
> > >        * hblk seems to be the line + porches length.
> > >        * The blank is set using a blanking packet (4 bytes + 4 bytes +
> > > @@ -502,6 +494,18 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> > >       hblk_max -= HBLK_PACKET_OVERHEAD;
> > >       hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max);
> > >
> > > +     /*
> > > +      * The frontporch is set using a blanking packet (4 bytes +
> > > +      * payload + 2 bytes). Its minimal size is therefore 6 bytes
> > > +      *
> > > +      * According to BSP code, extra 10 bytes(which is hblk packet overhead)
> > > +      * is adding for hfp packet overhead since hfp depends on hblk.
> >
> > "According to the BSP code, another 10 bytes (the HBLK packet
> > overhead) need to be added to the front porch overhead."
> >
> > How has this been verified? Since we have registers to set all the
> > parameters, this seems kind of weird.
> 
> Verified based on the BSP calculation which I explained in commit
> message and tested by validating these numbers with mainline vs bsp
> code.

So, not verified?

Did you have a panel that was broken due to this? Have you found that
the timings output on the DSI bus were wrong using an oscilloscope or
an analyzer? What makes you say that the Allwinner code is more
correct than we are?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v3 15/25] drm/sun4i: sun6i_mipi_dsi: Set proper vblk timing calculation

[Jagan Teki]

Unlike hblk, the vblk timings should follow an equation to compute
the desired value for lane 4 devices and rest of devices it would be 0.

BSP code from BPI-M64-bsp is computing vblk as for 4-lane devices
(in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
tmp = (ht*dsi_pixel_bits[format]/8)*vt-(4+dsi_hblk+2);
dsi_vblk = (lane-tmp%lane);

So, update the vblk timing calculation accordingly.

Tested on 2-lane, 4-lane MIPI-DSI LCD panels.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- new patch
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 20e330186b7f..42bd7506abaf 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -511,8 +511,19 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	 * Allwinner BSP is using a rather convoluted calculation
 	 * there only for 4 lanes. However, using 0 (the !4 lanes
 	 * case) even with a 4 lanes screen seems to work...
+	 *
+	 * The vertical blank is set using a blanking packet (4 bytes +
+	 * payload + 2 bytes). Its minimal size is therefore 6 bytes
 	 */
-	vblk = 0;
+#define VBLK_PACKET_OVERHEAD	6
+	if (device->lanes == 4) {
+		int tmp;
+
+		tmp = (mode->htotal * Bpp) * mode->vtotal - (hblk + VBLK_PACKET_OVERHEAD);
+		vblk =(device->lanes - tmp % device->lanes);
+	} else {
+		vblk = 0;
+	}
 
 	/* How many bytes do we need to send all payloads? */
 	bytes = max_t(size_t, max(max(hfp, hblk), max(hsa, hbp)), vblk);
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v3 15/25] drm/sun4i: sun6i_mipi_dsi: Set proper vblk timing calculation

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2046 bytes --]

On Fri, Oct 26, 2018 at 08:13:34PM +0530, Jagan Teki wrote:
> Unlike hblk, the vblk timings should follow an equation to compute
> the desired value for lane 4 devices and rest of devices it would be 0.
> 
> BSP code from BPI-M64-bsp is computing vblk as for 4-lane devices
> (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
> tmp = (ht*dsi_pixel_bits[format]/8)*vt-(4+dsi_hblk+2);
> dsi_vblk = (lane-tmp%lane);
> 
> So, update the vblk timing calculation accordingly.
> 
> Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - new patch
> Changes for v2:
> - none
> 
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 20e330186b7f..42bd7506abaf 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -511,8 +511,19 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
>  	 * Allwinner BSP is using a rather convoluted calculation
>  	 * there only for 4 lanes. However, using 0 (the !4 lanes
>  	 * case) even with a 4 lanes screen seems to work...

Since you're adding the support for 2 lanes DSI displays, that should
be removed.

> +	 *
> +	 * The vertical blank is set using a blanking packet (4 bytes +
> +	 * payload + 2 bytes). Its minimal size is therefore 6 bytes
>  	 */
> -	vblk = 0;
> +#define VBLK_PACKET_OVERHEAD	6
> +	if (device->lanes == 4) {
> +		int tmp;
> +
> +		tmp = (mode->htotal * Bpp) * mode->vtotal - (hblk + VBLK_PACKET_OVERHEAD);
> +		vblk =(device->lanes - tmp % device->lanes);

This generates a checkpatch warning. You should run all your patches
through checkpatch before sending them.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v3 16/25] drm/sun4i: sun6i_mipi_dsi: Add support for VCC-DSI voltage regulator

[Jagan Teki]

Some boards have VCC-DSI pin connected to voltage regulator which may
not be turned on by default.

Add support for such boards by adding voltage regulator handling code to
MIPI DSI driver.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- new patch
Changes for v2:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 14 ++++++++++++++
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 42bd7506abaf..bc57343592e0 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -949,6 +949,12 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
 
 	dsi->drv = drv;
 
+	ret = regulator_enable(dsi->regulator);
+	if (ret) {
+		dev_err(dev, "Failed to enable regulator\n");
+		return ret;
+	}
+
 	drm_encoder_helper_add(&dsi->encoder,
 			       &sun6i_dsi_enc_helper_funcs);
 	ret = drm_encoder_init(drm,
@@ -980,6 +986,7 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
 
 err_cleanup_connector:
 	drm_encoder_cleanup(&dsi->encoder);
+	regulator_disable(dsi->regulator);
 	return ret;
 }
 
@@ -989,6 +996,7 @@ static void sun6i_dsi_unbind(struct device *dev, struct device *master,
 	struct sun6i_dsi *dsi = dev_get_drvdata(dev);
 
 	drm_panel_detach(dsi->panel);
+	regulator_disable(dsi->regulator);
 }
 
 static const struct component_ops sun6i_dsi_ops = {
@@ -1022,6 +1030,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 		return PTR_ERR(base);
 	}
 
+	dsi->regulator = devm_regulator_get(dev, "vcc-dsi");
+	if (IS_ERR(dsi->regulator)) {
+		dev_err(dev, "Couldn't get regulator\n");
+		return PTR_ERR(dsi->regulator);
+	}
+
 	dsi->regs = devm_regmap_init_mmio_clk(dev, "bus", base,
 					      &sun6i_dsi_regmap_config);
 	if (IS_ERR(dsi->regs)) {
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index 597b62227019..0df60f84bab3 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -13,6 +13,8 @@
 #include <drm/drm_encoder.h>
 #include <drm/drm_mipi_dsi.h>
 
+#include <linux/regulator/consumer.h>
+
 struct sun6i_dphy {
 	struct clk		*bus_clk;
 	struct clk		*mod_clk;
@@ -32,6 +34,7 @@ struct sun6i_dsi {
 	struct clk		*bus_clk;
 	struct clk		*mod_clk;
 	struct regmap		*regs;
+	struct regulator	*regulator;
 	struct reset_control	*reset;
 	struct sun6i_dphy	*dphy;
 
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v3 16/25] drm/sun4i: sun6i_mipi_dsi: Add support for VCC-DSI voltage regulator

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2297 bytes --]

On Fri, Oct 26, 2018 at 08:13:35PM +0530, Jagan Teki wrote:
> Some boards have VCC-DSI pin connected to voltage regulator which may
> not be turned on by default.
> 
> Add support for such boards by adding voltage regulator handling code to
> MIPI DSI driver.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - new patch
> Changes for v2:
> - none
> 
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 14 ++++++++++++++
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  3 +++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 42bd7506abaf..bc57343592e0 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -949,6 +949,12 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
>  
>  	dsi->drv = drv;
>  
> +	ret = regulator_enable(dsi->regulator);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable regulator\n");
> +		return ret;
> +	}
> +

The regulator should be enabled only when the device is in use.

>  	drm_encoder_helper_add(&dsi->encoder,
>  			       &sun6i_dsi_enc_helper_funcs);
>  	ret = drm_encoder_init(drm,
> @@ -980,6 +986,7 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
>  
>  err_cleanup_connector:
>  	drm_encoder_cleanup(&dsi->encoder);
> +	regulator_disable(dsi->regulator);
>  	return ret;
>  }
>  
> @@ -989,6 +996,7 @@ static void sun6i_dsi_unbind(struct device *dev, struct device *master,
>  	struct sun6i_dsi *dsi = dev_get_drvdata(dev);
>  
>  	drm_panel_detach(dsi->panel);
> +	regulator_disable(dsi->regulator);
>  }
>  
>  static const struct component_ops sun6i_dsi_ops = {
> @@ -1022,6 +1030,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>  		return PTR_ERR(base);
>  	}
>  
> +	dsi->regulator = devm_regulator_get(dev, "vcc-dsi");
> +	if (IS_ERR(dsi->regulator)) {
> +		dev_err(dev, "Couldn't get regulator\n");
> +		return PTR_ERR(dsi->regulator);
> +	}
> +

You haven't updated the DT bindings accordingly.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 16/25] drm/sun4i: sun6i_mipi_dsi: Add support for VCC-DSI voltage regulator

[Jagan Teki]

On Mon, Oct 29, 2018 at 3:01 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Fri, Oct 26, 2018 at 08:13:35PM +0530, Jagan Teki wrote:
> > Some boards have VCC-DSI pin connected to voltage regulator which may
> > not be turned on by default.
> >
> > Add support for such boards by adding voltage regulator handling code to
> > MIPI DSI driver.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v3:
> > - new patch
> > Changes for v2:
> > - none
> >
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 14 ++++++++++++++
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  3 +++
> >  2 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 42bd7506abaf..bc57343592e0 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -949,6 +949,12 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
> >
> >       dsi->drv = drv;
> >
> > +     ret = regulator_enable(dsi->regulator);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to enable regulator\n");
> > +             return ret;
> > +     }
> > +
>
> The regulator should be enabled only when the device is in use.

True, but there is no device specific quirk needed because who ever
not needed simply use dummy regulator
[    0.245821] sun6i-mipi-dsi 1ca0000.dsi: 1ca0000.dsi supply vcc-dsi
not found, using dummy regulato
r

even like HVCC in hdmi driver [1]

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c?id=633ba1e086e1abbeef1ffd899911de8cf3987d9f

Re: [PATCH v3 16/25] drm/sun4i: sun6i_mipi_dsi: Add support for VCC-DSI voltage regulator

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2172 bytes --]

On Mon, Oct 29, 2018 at 08:18:05PM +0530, Jagan Teki wrote:
> On Mon, Oct 29, 2018 at 3:01 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Fri, Oct 26, 2018 at 08:13:35PM +0530, Jagan Teki wrote:
> > > Some boards have VCC-DSI pin connected to voltage regulator which may
> > > not be turned on by default.
> > >
> > > Add support for such boards by adding voltage regulator handling code to
> > > MIPI DSI driver.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > > Changes for v3:
> > > - new patch
> > > Changes for v2:
> > > - none
> > >
> > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 14 ++++++++++++++
> > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  3 +++
> > >  2 files changed, 17 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > index 42bd7506abaf..bc57343592e0 100644
> > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > @@ -949,6 +949,12 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
> > >
> > >       dsi->drv = drv;
> > >
> > > +     ret = regulator_enable(dsi->regulator);
> > > +     if (ret) {
> > > +             dev_err(dev, "Failed to enable regulator\n");
> > > +             return ret;
> > > +     }
> > > +
> >
> > The regulator should be enabled only when the device is in use.
> 
> True, but there is no device specific quirk needed because who ever
> not needed simply use dummy regulator
> [    0.245821] sun6i-mipi-dsi 1ca0000.dsi: 1ca0000.dsi supply vcc-dsi
> not found, using dummy regulato
> r
> 
> even like HVCC in hdmi driver [1]
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c?id=633ba1e086e1abbeef1ffd899911de8cf3987d9f

That's not my point. You should enable the regulator only when the DSI
bridge is being enabled, not when the driver has probed.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Jagan Teki]

Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
bridge panel, which is available on same PCB with 24-bit RGB interface.

So, this patch adds DSI specific binding details on existing
dt-bindings file.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- Use existing binding doc and update dsi details
Changes for v2:
- none

 .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
index 35bc0c839f49..b7855dc7c66f 100644
--- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
+++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
@@ -1,12 +1,39 @@
 Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
 
+S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
+
+Depending on the variant, the PCB attached to the panel module either
+supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
+an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
+itself
+
 Required properties:
-- compatible: should be "bananapi,s070wv20-ct16"
+- compatible:
+  for 24-bit RGB interface, use "bananapi,s070wv20-ct16"
+  for ICN6211 MIPI-DSI to RGB bridge, use "bananapi,s070wv20-ct16-icn6211"
+
+Required properties for RGB:
 - power-supply: see ./panel-common.txt
 
+Required properties for MIPI-DSI to RGB:
+- reg: for DSI virtual channel used by that screen
+- avdd-supply: analog regulator dc1 switch
+- dvdd-supply: 3v3 digital regulator
+- reset-gpios: a GPIO phandle for the reset pin
+
 Optional properties:
-- enable-gpios: see ./simple-panel.txt
+- enable-gpios: see ./simple-panel.txt(not available in MIPI-DSI to RGB bridge)
 - backlight: see ./simple-panel.txt
 
 This binding is compatible with the simple-panel binding, which is specified
 in ./simple-panel.txt.
+
+Example:
+panel@0 {
+	compatible = "bananapi,s070wv20-ct16-icn6211";
+	reg = <0>;
+	avdd-supply = <&reg_dc1sw>;
+	dvdd-supply = <&reg_dldo1>;
+	reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+	backlight = <&backlight_dsi>;
+};
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Rob Herring]

On Fri, 26 Oct 2018 20:13:36 +0530, Jagan Teki wrote:
> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
> bridge panel, which is available on same PCB with 24-bit RGB interface.
> 
> So, this patch adds DSI specific binding details on existing
> dt-bindings file.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - Use existing binding doc and update dsi details
> Changes for v2:
> - none
> 
>  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 

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

Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Andrzej Hajda]

On 26.10.2018 16:43, Jagan Teki wrote:
> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
> bridge panel, which is available on same PCB with 24-bit RGB interface.
>
> So, this patch adds DSI specific binding details on existing
> dt-bindings file.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - Use existing binding doc and update dsi details
> Changes for v2:
> - none
>
>  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> index 35bc0c839f49..b7855dc7c66f 100644
> --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> @@ -1,12 +1,39 @@
>  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
>  
> +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
> +
> +Depending on the variant, the PCB attached to the panel module either
> +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
> +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
> +itself

As I understand this is display board, which contains 'pure' RGB panel
S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
These are separate devices, just connected by vendor to simplify its
assembly. Why don't you create then bridge driver for ICN6211 and RGB
panel driver for S070WV20-CT16 - it looks more generic.
Then you can describe both in dts and voila.
Creating drivers for every combo of devices (panel + bridge), just
because some vendor sells them together seems incorrect - we have
devicetree for it.

Regards
Andrzej


> +
>  Required properties:
> -- compatible: should be "bananapi,s070wv20-ct16"
> +- compatible:
> +  for 24-bit RGB interface, use "bananapi,s070wv20-ct16"
> +  for ICN6211 MIPI-DSI to RGB bridge, use "bananapi,s070wv20-ct16-icn6211"
> +
> +Required properties for RGB:
>  - power-supply: see ./panel-common.txt
>  
> +Required properties for MIPI-DSI to RGB:
> +- reg: for DSI virtual channel used by that screen
> +- avdd-supply: analog regulator dc1 switch
> +- dvdd-supply: 3v3 digital regulator
> +- reset-gpios: a GPIO phandle for the reset pin
> +
>  Optional properties:
> -- enable-gpios: see ./simple-panel.txt
> +- enable-gpios: see ./simple-panel.txt(not available in MIPI-DSI to RGB bridge)
>  - backlight: see ./simple-panel.txt
>  
>  This binding is compatible with the simple-panel binding, which is specified
>  in ./simple-panel.txt.
> +
> +Example:
> +panel@0 {
> +	compatible = "bananapi,s070wv20-ct16-icn6211";
> +	reg = <0>;
> +	avdd-supply = <&reg_dc1sw>;
> +	dvdd-supply = <&reg_dldo1>;
> +	reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> +	backlight = <&backlight_dsi>;
> +};

Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Chen-Yu Tsai]

On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>
> On 26.10.2018 16:43, Jagan Teki wrote:
> > Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
> > bridge panel, which is available on same PCB with 24-bit RGB interface.
> >
> > So, this patch adds DSI specific binding details on existing
> > dt-bindings file.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v3:
> > - Use existing binding doc and update dsi details
> > Changes for v2:
> > - none
> >
> >  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
> >  1 file changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > index 35bc0c839f49..b7855dc7c66f 100644
> > --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > @@ -1,12 +1,39 @@
> >  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
> >
> > +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
> > +
> > +Depending on the variant, the PCB attached to the panel module either
> > +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
> > +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
> > +itself
>
> As I understand this is display board, which contains 'pure' RGB panel
> S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
> These are separate devices, just connected by vendor to simplify its
> assembly. Why don't you create then bridge driver for ICN6211 and RGB
> panel driver for S070WV20-CT16 - it looks more generic.
> Then you can describe both in dts and voila.
> Creating drivers for every combo of devices (panel + bridge), just
> because some vendor sells them together seems incorrect - we have
> devicetree for it.

Rob suggested this, and also the opposite: using the same
"bananapi,s070wv20-ct16"
compatible string for both types of connections, and have the driver deal with
detecting the bus type.

The thing about the bridge chip is that there's no available datasheet that
describes all the parts of the init sequence, in fact none at all. I managed
to work out some bits, but the others remain a mystery and must be hard-coded
to match the panel. That would work against having a generic bridge driver.

ChenYu

>
> Regards
> Andrzej
>
>
> > +
> >  Required properties:
> > -- compatible: should be "bananapi,s070wv20-ct16"
> > +- compatible:
> > +  for 24-bit RGB interface, use "bananapi,s070wv20-ct16"
> > +  for ICN6211 MIPI-DSI to RGB bridge, use "bananapi,s070wv20-ct16-icn6211"
> > +
> > +Required properties for RGB:
> >  - power-supply: see ./panel-common.txt
> >
> > +Required properties for MIPI-DSI to RGB:
> > +- reg: for DSI virtual channel used by that screen
> > +- avdd-supply: analog regulator dc1 switch
> > +- dvdd-supply: 3v3 digital regulator
> > +- reset-gpios: a GPIO phandle for the reset pin
> > +
> >  Optional properties:
> > -- enable-gpios: see ./simple-panel.txt
> > +- enable-gpios: see ./simple-panel.txt(not available in MIPI-DSI to RGB bridge)
> >  - backlight: see ./simple-panel.txt
> >
> >  This binding is compatible with the simple-panel binding, which is specified
> >  in ./simple-panel.txt.
> > +
> > +Example:
> > +panel@0 {
> > +     compatible = "bananapi,s070wv20-ct16-icn6211";
> > +     reg = <0>;
> > +     avdd-supply = <&reg_dc1sw>;
> > +     dvdd-supply = <&reg_dldo1>;
> > +     reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> > +     backlight = <&backlight_dsi>;
> > +};
>
>

Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Andrzej Hajda]

On 31.10.2018 09:58, Chen-Yu Tsai wrote:
> On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>> On 26.10.2018 16:43, Jagan Teki wrote:
>>> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
>>> bridge panel, which is available on same PCB with 24-bit RGB interface.
>>>
>>> So, this patch adds DSI specific binding details on existing
>>> dt-bindings file.
>>>
>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>> ---
>>> Changes for v3:
>>> - Use existing binding doc and update dsi details
>>> Changes for v2:
>>> - none
>>>
>>>  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
>>>  1 file changed, 29 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>> index 35bc0c839f49..b7855dc7c66f 100644
>>> --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>> +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>> @@ -1,12 +1,39 @@
>>>  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
>>>
>>> +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
>>> +
>>> +Depending on the variant, the PCB attached to the panel module either
>>> +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
>>> +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
>>> +itself
>> As I understand this is display board, which contains 'pure' RGB panel
>> S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
>> These are separate devices, just connected by vendor to simplify its
>> assembly. Why don't you create then bridge driver for ICN6211 and RGB
>> panel driver for S070WV20-CT16 - it looks more generic.
>> Then you can describe both in dts and voila.
>> Creating drivers for every combo of devices (panel + bridge), just
>> because some vendor sells them together seems incorrect - we have
>> devicetree for it.
> Rob suggested this, and also the opposite: using the same
> "bananapi,s070wv20-ct16"
> compatible string for both types of connections, and have the driver deal with
> detecting the bus type.
>
> The thing about the bridge chip is that there's no available datasheet that
> describes all the parts of the init sequence, in fact none at all. I managed
> to work out some bits, but the others remain a mystery and must be hard-coded
> to match the panel. That would work against having a generic bridge driver.


But it is common for many chips - 1st version of the driver is developed
on one platform and it supports only one configuration, if next platform
with the same cheap appears the driver is augmented if necessary.

Regards

Andrzej


>
> ChenYu
>
>> Regards
>> Andrzej
>>
>>
>>> +
>>>  Required properties:
>>> -- compatible: should be "bananapi,s070wv20-ct16"
>>> +- compatible:
>>> +  for 24-bit RGB interface, use "bananapi,s070wv20-ct16"
>>> +  for ICN6211 MIPI-DSI to RGB bridge, use "bananapi,s070wv20-ct16-icn6211"
>>> +
>>> +Required properties for RGB:
>>>  - power-supply: see ./panel-common.txt
>>>
>>> +Required properties for MIPI-DSI to RGB:
>>> +- reg: for DSI virtual channel used by that screen
>>> +- avdd-supply: analog regulator dc1 switch
>>> +- dvdd-supply: 3v3 digital regulator
>>> +- reset-gpios: a GPIO phandle for the reset pin
>>> +
>>>  Optional properties:
>>> -- enable-gpios: see ./simple-panel.txt
>>> +- enable-gpios: see ./simple-panel.txt(not available in MIPI-DSI to RGB bridge)
>>>  - backlight: see ./simple-panel.txt
>>>
>>>  This binding is compatible with the simple-panel binding, which is specified
>>>  in ./simple-panel.txt.
>>> +
>>> +Example:
>>> +panel@0 {
>>> +     compatible = "bananapi,s070wv20-ct16-icn6211";
>>> +     reg = <0>;
>>> +     avdd-supply = <&reg_dc1sw>;
>>> +     dvdd-supply = <&reg_dldo1>;
>>> +     reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
>>> +     backlight = <&backlight_dsi>;
>>> +};
>>
>

Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Jagan Teki]

On Wed, Oct 31, 2018 at 2:45 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>
> On 31.10.2018 09:58, Chen-Yu Tsai wrote:
> > On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
> >> On 26.10.2018 16:43, Jagan Teki wrote:
> >>> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
> >>> bridge panel, which is available on same PCB with 24-bit RGB interface.
> >>>
> >>> So, this patch adds DSI specific binding details on existing
> >>> dt-bindings file.
> >>>
> >>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >>> ---
> >>> Changes for v3:
> >>> - Use existing binding doc and update dsi details
> >>> Changes for v2:
> >>> - none
> >>>
> >>>  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
> >>>  1 file changed, 29 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> >>> index 35bc0c839f49..b7855dc7c66f 100644
> >>> --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> >>> +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> >>> @@ -1,12 +1,39 @@
> >>>  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
> >>>
> >>> +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
> >>> +
> >>> +Depending on the variant, the PCB attached to the panel module either
> >>> +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
> >>> +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
> >>> +itself
> >> As I understand this is display board, which contains 'pure' RGB panel
> >> S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
> >> These are separate devices, just connected by vendor to simplify its
> >> assembly. Why don't you create then bridge driver for ICN6211 and RGB
> >> panel driver for S070WV20-CT16 - it looks more generic.
> >> Then you can describe both in dts and voila.
> >> Creating drivers for every combo of devices (panel + bridge), just
> >> because some vendor sells them together seems incorrect - we have
> >> devicetree for it.
> > Rob suggested this, and also the opposite: using the same
> > "bananapi,s070wv20-ct16"
> > compatible string for both types of connections, and have the driver deal with
> > detecting the bus type.
> >
> > The thing about the bridge chip is that there's no available datasheet that
> > describes all the parts of the init sequence, in fact none at all. I managed
> > to work out some bits, but the others remain a mystery and must be hard-coded
> > to match the panel. That would work against having a generic bridge driver.
>
>
> But it is common for many chips - 1st version of the driver is developed
> on one platform and it supports only one configuration, if next platform
> with the same cheap appears the driver is augmented if necessary.

At-least few of the commands from panel initialization code, the
respective opcode data values are based on panel timings and even
clock value is different in DSI. I think it look hard to try bridge
driver for these restrictions, do you have any suggestions?

Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Andrzej Hajda]

On 06.11.2018 19:08, Jagan Teki wrote:
> On Wed, Oct 31, 2018 at 2:45 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>> On 31.10.2018 09:58, Chen-Yu Tsai wrote:
>>> On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>>>> On 26.10.2018 16:43, Jagan Teki wrote:
>>>>> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
>>>>> bridge panel, which is available on same PCB with 24-bit RGB interface.
>>>>>
>>>>> So, this patch adds DSI specific binding details on existing
>>>>> dt-bindings file.
>>>>>
>>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>>> ---
>>>>> Changes for v3:
>>>>> - Use existing binding doc and update dsi details
>>>>> Changes for v2:
>>>>> - none
>>>>>
>>>>>  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
>>>>>  1 file changed, 29 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>>>> index 35bc0c839f49..b7855dc7c66f 100644
>>>>> --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>>>> +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>>>> @@ -1,12 +1,39 @@
>>>>>  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
>>>>>
>>>>> +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
>>>>> +
>>>>> +Depending on the variant, the PCB attached to the panel module either
>>>>> +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
>>>>> +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
>>>>> +itself
>>>> As I understand this is display board, which contains 'pure' RGB panel
>>>> S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
>>>> These are separate devices, just connected by vendor to simplify its
>>>> assembly. Why don't you create then bridge driver for ICN6211 and RGB
>>>> panel driver for S070WV20-CT16 - it looks more generic.
>>>> Then you can describe both in dts and voila.
>>>> Creating drivers for every combo of devices (panel + bridge), just
>>>> because some vendor sells them together seems incorrect - we have
>>>> devicetree for it.
>>> Rob suggested this, and also the opposite: using the same
>>> "bananapi,s070wv20-ct16"
>>> compatible string for both types of connections, and have the driver deal with
>>> detecting the bus type.
>>>
>>> The thing about the bridge chip is that there's no available datasheet that
>>> describes all the parts of the init sequence, in fact none at all. I managed
>>> to work out some bits, but the others remain a mystery and must be hard-coded
>>> to match the panel. That would work against having a generic bridge driver.
>>
>> But it is common for many chips - 1st version of the driver is developed
>> on one platform and it supports only one configuration, if next platform
>> with the same cheap appears the driver is augmented if necessary.
> At-least few of the commands from panel initialization code, the
> respective opcode data values are based on panel timings and even
> clock value is different in DSI. I think it look hard to try bridge
> driver for these restrictions, do you have any suggestions?


Where do you see an issue? Since panel is RGB it should have no
initialization sequence (beside regulator/gpio power on/off), so the
only thing to do is to figure out which regulators/gpios belongs to
which component - with publicly available specs it should be doable.

The whole initialization sequence is for the bridge, so you put it into
bridge driver, for starters it can be hardcoded.

Then you can:

1. Try to find other users of this ICN6211 chip and compare
initialization sequences to guess purpose of registers.

2. Try to get specs of the chip (ask vendor, distributor, grep Internet).

3. Do nothing - if there will be other users of the bridge they will do
this work.


Regards

Andrzej

Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Jagan Teki]

On Wed, Nov 7, 2018 at 2:41 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>
> On 06.11.2018 19:08, Jagan Teki wrote:
> > On Wed, Oct 31, 2018 at 2:45 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
> >> On 31.10.2018 09:58, Chen-Yu Tsai wrote:
> >>> On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
> >>>> On 26.10.2018 16:43, Jagan Teki wrote:
> >>>>> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
> >>>>> bridge panel, which is available on same PCB with 24-bit RGB interface.
> >>>>>
> >>>>> So, this patch adds DSI specific binding details on existing
> >>>>> dt-bindings file.
> >>>>>
> >>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >>>>> ---
> >>>>> Changes for v3:
> >>>>> - Use existing binding doc and update dsi details
> >>>>> Changes for v2:
> >>>>> - none
> >>>>>
> >>>>>  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
> >>>>>  1 file changed, 29 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> >>>>> index 35bc0c839f49..b7855dc7c66f 100644
> >>>>> --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> >>>>> +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> >>>>> @@ -1,12 +1,39 @@
> >>>>>  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
> >>>>>
> >>>>> +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
> >>>>> +
> >>>>> +Depending on the variant, the PCB attached to the panel module either
> >>>>> +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
> >>>>> +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
> >>>>> +itself
> >>>> As I understand this is display board, which contains 'pure' RGB panel
> >>>> S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
> >>>> These are separate devices, just connected by vendor to simplify its
> >>>> assembly. Why don't you create then bridge driver for ICN6211 and RGB
> >>>> panel driver for S070WV20-CT16 - it looks more generic.
> >>>> Then you can describe both in dts and voila.
> >>>> Creating drivers for every combo of devices (panel + bridge), just
> >>>> because some vendor sells them together seems incorrect - we have
> >>>> devicetree for it.
> >>> Rob suggested this, and also the opposite: using the same
> >>> "bananapi,s070wv20-ct16"
> >>> compatible string for both types of connections, and have the driver deal with
> >>> detecting the bus type.
> >>>
> >>> The thing about the bridge chip is that there's no available datasheet that
> >>> describes all the parts of the init sequence, in fact none at all. I managed
> >>> to work out some bits, but the others remain a mystery and must be hard-coded
> >>> to match the panel. That would work against having a generic bridge driver.
> >>
> >> But it is common for many chips - 1st version of the driver is developed
> >> on one platform and it supports only one configuration, if next platform
> >> with the same cheap appears the driver is augmented if necessary.
> > At-least few of the commands from panel initialization code, the
> > respective opcode data values are based on panel timings and even
> > clock value is different in DSI. I think it look hard to try bridge
> > driver for these restrictions, do you have any suggestions?
>
>
> Where do you see an issue? Since panel is RGB it should have no
> initialization sequence (beside regulator/gpio power on/off), so the
> only thing to do is to figure out which regulators/gpios belongs to
> which component - with publicly available specs it should be doable.
>
> The whole initialization sequence is for the bridge, so you put it into
> bridge driver, for starters it can be hardcoded.

Yes, I understand we can move regulators/gpio setup separately and
though we hardcode the init sequence there is difference  in clock for
DSI(which I mentioned in previous mail). DSI panel can't work with
clock used by RGB panel-simple.

>
> Then you can:
>
> 1. Try to find other users of this ICN6211 chip and compare
> initialization sequences to guess purpose of registers.
>
> 2. Try to get specs of the chip (ask vendor, distributor, grep Internet).

As we mentioned (even Chen-Yu), we are unable to find the proper spec
for this panel, all we taken reference from AW BSP code.

>
> 3. Do nothing - if there will be other users of the bridge they will do
> this work.

Don't know how we can go with generic bridge driver irrespective of
these particular wrinkles, let me know if you have any suggestions.

Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Andrzej Hajda]

On 10.11.2018 08:32, Jagan Teki wrote:
> On Wed, Nov 7, 2018 at 2:41 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>> On 06.11.2018 19:08, Jagan Teki wrote:
>>> On Wed, Oct 31, 2018 at 2:45 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>>>> On 31.10.2018 09:58, Chen-Yu Tsai wrote:
>>>>> On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>>>>>> On 26.10.2018 16:43, Jagan Teki wrote:
>>>>>>> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
>>>>>>> bridge panel, which is available on same PCB with 24-bit RGB interface.
>>>>>>>
>>>>>>> So, this patch adds DSI specific binding details on existing
>>>>>>> dt-bindings file.
>>>>>>>
>>>>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>>>>> ---
>>>>>>> Changes for v3:
>>>>>>> - Use existing binding doc and update dsi details
>>>>>>> Changes for v2:
>>>>>>> - none
>>>>>>>
>>>>>>>  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
>>>>>>>  1 file changed, 29 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>>>>>> index 35bc0c839f49..b7855dc7c66f 100644
>>>>>>> --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>>>>>> +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>>>>>> @@ -1,12 +1,39 @@
>>>>>>>  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
>>>>>>>
>>>>>>> +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
>>>>>>> +
>>>>>>> +Depending on the variant, the PCB attached to the panel module either
>>>>>>> +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
>>>>>>> +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
>>>>>>> +itself
>>>>>> As I understand this is display board, which contains 'pure' RGB panel
>>>>>> S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
>>>>>> These are separate devices, just connected by vendor to simplify its
>>>>>> assembly. Why don't you create then bridge driver for ICN6211 and RGB
>>>>>> panel driver for S070WV20-CT16 - it looks more generic.
>>>>>> Then you can describe both in dts and voila.
>>>>>> Creating drivers for every combo of devices (panel + bridge), just
>>>>>> because some vendor sells them together seems incorrect - we have
>>>>>> devicetree for it.
>>>>> Rob suggested this, and also the opposite: using the same
>>>>> "bananapi,s070wv20-ct16"
>>>>> compatible string for both types of connections, and have the driver deal with
>>>>> detecting the bus type.
>>>>>
>>>>> The thing about the bridge chip is that there's no available datasheet that
>>>>> describes all the parts of the init sequence, in fact none at all. I managed
>>>>> to work out some bits, but the others remain a mystery and must be hard-coded
>>>>> to match the panel. That would work against having a generic bridge driver.
>>>> But it is common for many chips - 1st version of the driver is developed
>>>> on one platform and it supports only one configuration, if next platform
>>>> with the same cheap appears the driver is augmented if necessary.
>>> At-least few of the commands from panel initialization code, the
>>> respective opcode data values are based on panel timings and even
>>> clock value is different in DSI. I think it look hard to try bridge
>>> driver for these restrictions, do you have any suggestions?
>>
>> Where do you see an issue? Since panel is RGB it should have no
>> initialization sequence (beside regulator/gpio power on/off), so the
>> only thing to do is to figure out which regulators/gpios belongs to
>> which component - with publicly available specs it should be doable.
>>
>> The whole initialization sequence is for the bridge, so you put it into
>> bridge driver, for starters it can be hardcoded.
> Yes, I understand we can move regulators/gpio setup separately and
> though we hardcode the init sequence there is difference  in clock for
> DSI(which I mentioned in previous mail). DSI panel can't work with
> clock used by RGB panel-simple.


If you mean pixel clock from timings in next patch it seems incorrect.
Pixel clock should be always

htotal * vtotal * vrefresh, in case of drm_display_mode result should be
divided by 1000 (as .clock is in kHz).

With timings provided there you have: 928*525*60 = 29232000

So pixel clock should be 29232, if other timings are correct. DSI clock
is a different thing and it is private thing of DSI bridge/panel it
should not be exposed via drm_display_mode.


Regards

Andrzej


>
>> Then you can:
>>
>> 1. Try to find other users of this ICN6211 chip and compare
>> initialization sequences to guess purpose of registers.
>>
>> 2. Try to get specs of the chip (ask vendor, distributor, grep Internet).
> As we mentioned (even Chen-Yu), we are unable to find the proper spec
> for this panel, all we taken reference from AW BSP code.
>
>> 3. Do nothing - if there will be other users of the bridge they will do
>> this work.
> Don't know how we can go with generic bridge driver irrespective of
> these particular wrinkles, let me know if you have any suggestions.
>
>

Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Jagan Teki]

On Tue, Nov 13, 2018 at 1:26 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>
> On 10.11.2018 08:32, Jagan Teki wrote:
> > On Wed, Nov 7, 2018 at 2:41 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
> >> On 06.11.2018 19:08, Jagan Teki wrote:
> >>> On Wed, Oct 31, 2018 at 2:45 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
> >>>> On 31.10.2018 09:58, Chen-Yu Tsai wrote:
> >>>>> On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
> >>>>>> On 26.10.2018 16:43, Jagan Teki wrote:
> >>>>>>> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
> >>>>>>> bridge panel, which is available on same PCB with 24-bit RGB interface.
> >>>>>>>
> >>>>>>> So, this patch adds DSI specific binding details on existing
> >>>>>>> dt-bindings file.
> >>>>>>>
> >>>>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >>>>>>> ---
> >>>>>>> Changes for v3:
> >>>>>>> - Use existing binding doc and update dsi details
> >>>>>>> Changes for v2:
> >>>>>>> - none
> >>>>>>>
> >>>>>>>  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
> >>>>>>>  1 file changed, 29 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> >>>>>>> index 35bc0c839f49..b7855dc7c66f 100644
> >>>>>>> --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> >>>>>>> +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> >>>>>>> @@ -1,12 +1,39 @@
> >>>>>>>  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
> >>>>>>>
> >>>>>>> +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
> >>>>>>> +
> >>>>>>> +Depending on the variant, the PCB attached to the panel module either
> >>>>>>> +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
> >>>>>>> +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
> >>>>>>> +itself
> >>>>>> As I understand this is display board, which contains 'pure' RGB panel
> >>>>>> S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
> >>>>>> These are separate devices, just connected by vendor to simplify its
> >>>>>> assembly. Why don't you create then bridge driver for ICN6211 and RGB
> >>>>>> panel driver for S070WV20-CT16 - it looks more generic.
> >>>>>> Then you can describe both in dts and voila.
> >>>>>> Creating drivers for every combo of devices (panel + bridge), just
> >>>>>> because some vendor sells them together seems incorrect - we have
> >>>>>> devicetree for it.
> >>>>> Rob suggested this, and also the opposite: using the same
> >>>>> "bananapi,s070wv20-ct16"
> >>>>> compatible string for both types of connections, and have the driver deal with
> >>>>> detecting the bus type.
> >>>>>
> >>>>> The thing about the bridge chip is that there's no available datasheet that
> >>>>> describes all the parts of the init sequence, in fact none at all. I managed
> >>>>> to work out some bits, but the others remain a mystery and must be hard-coded
> >>>>> to match the panel. That would work against having a generic bridge driver.
> >>>> But it is common for many chips - 1st version of the driver is developed
> >>>> on one platform and it supports only one configuration, if next platform
> >>>> with the same cheap appears the driver is augmented if necessary.
> >>> At-least few of the commands from panel initialization code, the
> >>> respective opcode data values are based on panel timings and even
> >>> clock value is different in DSI. I think it look hard to try bridge
> >>> driver for these restrictions, do you have any suggestions?
> >>
> >> Where do you see an issue? Since panel is RGB it should have no
> >> initialization sequence (beside regulator/gpio power on/off), so the
> >> only thing to do is to figure out which regulators/gpios belongs to
> >> which component - with publicly available specs it should be doable.
> >>
> >> The whole initialization sequence is for the bridge, so you put it into
> >> bridge driver, for starters it can be hardcoded.
> > Yes, I understand we can move regulators/gpio setup separately and
> > though we hardcode the init sequence there is difference  in clock for
> > DSI(which I mentioned in previous mail). DSI panel can't work with
> > clock used by RGB panel-simple.
>
>
> If you mean pixel clock from timings in next patch it seems incorrect.
> Pixel clock should be always
>
> htotal * vtotal * vrefresh, in case of drm_display_mode result should be
> divided by 1000 (as .clock is in kHz).
>
> With timings provided there you have: 928*525*60 = 29232000
>
> So pixel clock should be 29232, if other timings are correct. DSI clock
> is a different thing and it is private thing of DSI bridge/panel it
> should not be exposed via drm_display_mode.

I understand your point, but in Allwinner case DSI clock which is
named in tcon block can be computed using clock value from
drm_display_mode from panel driver.

DE -> Mixer -> tcon-> MIPI <--> panel

So, for the value
- 55MHz panel clock the computed divider for DSI clock in tcon [1] is
6 which is proper and working divider
- 30MHz panel clock the computed divider for DSI clock in tcon [2] is
11 which can't work for this specific panel.

I have verified other panels, the divider value work similar to this
computation and those panels work accordingly.  but this panel case
the divider computation look weird, so we have used working and know
clock. I'm thinking since this panel is of DSI to RGB bridge, so it
may require some higher frequency to work but I've no document to
confirm the same.

Hope this information help, let me know if you have any inputs.

[1] https://paste.ubuntu.com/p/drvzfHFMtY/
[2] https://paste.ubuntu.com/p/hz29CTJY2J/

Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Andrzej Hajda]

On 18.11.2018 19:20, Jagan Teki wrote:
> On Tue, Nov 13, 2018 at 1:26 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>> On 10.11.2018 08:32, Jagan Teki wrote:
>>> On Wed, Nov 7, 2018 at 2:41 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>>>> On 06.11.2018 19:08, Jagan Teki wrote:
>>>>> On Wed, Oct 31, 2018 at 2:45 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>>>>>> On 31.10.2018 09:58, Chen-Yu Tsai wrote:
>>>>>>> On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>>>>>>>> On 26.10.2018 16:43, Jagan Teki wrote:
>>>>>>>>> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
>>>>>>>>> bridge panel, which is available on same PCB with 24-bit RGB interface.
>>>>>>>>>
>>>>>>>>> So, this patch adds DSI specific binding details on existing
>>>>>>>>> dt-bindings file.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>>>>>>> ---
>>>>>>>>> Changes for v3:
>>>>>>>>> - Use existing binding doc and update dsi details
>>>>>>>>> Changes for v2:
>>>>>>>>> - none
>>>>>>>>>
>>>>>>>>>  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
>>>>>>>>>  1 file changed, 29 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>>>>>>>> index 35bc0c839f49..b7855dc7c66f 100644
>>>>>>>>> --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>>>>>>>> +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>>>>>>>>> @@ -1,12 +1,39 @@
>>>>>>>>>  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
>>>>>>>>>
>>>>>>>>> +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
>>>>>>>>> +
>>>>>>>>> +Depending on the variant, the PCB attached to the panel module either
>>>>>>>>> +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
>>>>>>>>> +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
>>>>>>>>> +itself
>>>>>>>> As I understand this is display board, which contains 'pure' RGB panel
>>>>>>>> S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
>>>>>>>> These are separate devices, just connected by vendor to simplify its
>>>>>>>> assembly. Why don't you create then bridge driver for ICN6211 and RGB
>>>>>>>> panel driver for S070WV20-CT16 - it looks more generic.
>>>>>>>> Then you can describe both in dts and voila.
>>>>>>>> Creating drivers for every combo of devices (panel + bridge), just
>>>>>>>> because some vendor sells them together seems incorrect - we have
>>>>>>>> devicetree for it.
>>>>>>> Rob suggested this, and also the opposite: using the same
>>>>>>> "bananapi,s070wv20-ct16"
>>>>>>> compatible string for both types of connections, and have the driver deal with
>>>>>>> detecting the bus type.
>>>>>>>
>>>>>>> The thing about the bridge chip is that there's no available datasheet that
>>>>>>> describes all the parts of the init sequence, in fact none at all. I managed
>>>>>>> to work out some bits, but the others remain a mystery and must be hard-coded
>>>>>>> to match the panel. That would work against having a generic bridge driver.
>>>>>> But it is common for many chips - 1st version of the driver is developed
>>>>>> on one platform and it supports only one configuration, if next platform
>>>>>> with the same cheap appears the driver is augmented if necessary.
>>>>> At-least few of the commands from panel initialization code, the
>>>>> respective opcode data values are based on panel timings and even
>>>>> clock value is different in DSI. I think it look hard to try bridge
>>>>> driver for these restrictions, do you have any suggestions?
>>>> Where do you see an issue? Since panel is RGB it should have no
>>>> initialization sequence (beside regulator/gpio power on/off), so the
>>>> only thing to do is to figure out which regulators/gpios belongs to
>>>> which component - with publicly available specs it should be doable.
>>>>
>>>> The whole initialization sequence is for the bridge, so you put it into
>>>> bridge driver, for starters it can be hardcoded.
>>> Yes, I understand we can move regulators/gpio setup separately and
>>> though we hardcode the init sequence there is difference  in clock for
>>> DSI(which I mentioned in previous mail). DSI panel can't work with
>>> clock used by RGB panel-simple.
>>
>> If you mean pixel clock from timings in next patch it seems incorrect.
>> Pixel clock should be always
>>
>> htotal * vtotal * vrefresh, in case of drm_display_mode result should be
>> divided by 1000 (as .clock is in kHz).
>>
>> With timings provided there you have: 928*525*60 = 29232000
>>
>> So pixel clock should be 29232, if other timings are correct. DSI clock
>> is a different thing and it is private thing of DSI bridge/panel it
>> should not be exposed via drm_display_mode.
> I understand your point, but in Allwinner case DSI clock which is
> named in tcon block can be computed using clock value from
> drm_display_mode from panel driver.


OK, it sounds like me telling you that 2+2=4 and you answering that you
understand my point but in your case 2+2=5 fits better.

This is nothing about 'my point', this is simple fact that if you have
928*525 pixels (including hidden ones) with refresh rate 60Hz pixelclock
is 29232kHz.


>
> DE -> Mixer -> tcon-> MIPI <--> panel
>
> So, for the value
> - 55MHz panel clock the computed divider for DSI clock in tcon [1] is
> 6 which is proper and working divider
> - 30MHz panel clock the computed divider for DSI clock in tcon [2] is
> 11 which can't work for this specific panel.


Please explain what exactly means "panel clock" and the divider here.

What rate reports modetest -v ...? For this and other panels.

Maybe we can figure out what should be fixed/adjusted.


Regards

Andrzej


>
> I have verified other panels, the divider value work similar to this
> computation and those panels work accordingly.  but this panel case
> the divider computation look weird, so we have used working and know
> clock. I'm thinking since this panel is of DSI to RGB bridge, so it
> may require some higher frequency to work but I've no document to
> confirm the same.
>
> Hope this information help, let me know if you have any inputs.
>
> [1] https://paste.ubuntu.com/p/drvzfHFMtY/
> [2] https://paste.ubuntu.com/p/hz29CTJY2J/
>
>

Re: [linux-sunxi] Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Julian Calaby]

Hi Jagan,

On Wed, Oct 31, 2018 at 7:58 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
> >
> > On 26.10.2018 16:43, Jagan Teki wrote:
> > > Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
> > > bridge panel, which is available on same PCB with 24-bit RGB interface.
> > >
> > > So, this patch adds DSI specific binding details on existing
> > > dt-bindings file.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > > Changes for v3:
> > > - Use existing binding doc and update dsi details
> > > Changes for v2:
> > > - none
> > >
> > >  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
> > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > > index 35bc0c839f49..b7855dc7c66f 100644
> > > --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > > +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > > @@ -1,12 +1,39 @@
> > >  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
> > >
> > > +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
> > > +
> > > +Depending on the variant, the PCB attached to the panel module either
> > > +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
> > > +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
> > > +itself
> >
> > As I understand this is display board, which contains 'pure' RGB panel
> > S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
> > These are separate devices, just connected by vendor to simplify its
> > assembly. Why don't you create then bridge driver for ICN6211 and RGB
> > panel driver for S070WV20-CT16 - it looks more generic.
> > Then you can describe both in dts and voila.
> > Creating drivers for every combo of devices (panel + bridge), just
> > because some vendor sells them together seems incorrect - we have
> > devicetree for it.
>
> Rob suggested this, and also the opposite: using the same
> "bananapi,s070wv20-ct16"
> compatible string for both types of connections, and have the driver deal with
> detecting the bus type.
>
> The thing about the bridge chip is that there's no available datasheet that
> describes all the parts of the init sequence, in fact none at all. I managed
> to work out some bits, but the others remain a mystery and must be hard-coded
> to match the panel. That would work against having a generic bridge driver.

To me it seems logical that we'd model it as another step in the graph
between the DSI component and the panel. It's conceivable that some
other manufacturer will probably buy these for their panels and having
a somewhat generic driver seems vaguely future proof to me.

As I see it, the weird init process belongs to the bridge chip and the
timings belong to the panel and we shouldn't "confuse" them by giving
them the same compatible.

That said, I'm sure that these chips are already old hat and we'll
have something different and even more incomprehensible next week.

Thanks,

--
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [linux-sunxi] Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Jagan Teki]

On Wed, Oct 31, 2018 at 2:46 PM Julian Calaby <julian.calaby@gmail.com> wrote:
>
> Hi Jagan,
>
> On Wed, Oct 31, 2018 at 7:58 PM Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
> > >
> > > On 26.10.2018 16:43, Jagan Teki wrote:
> > > > Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
> > > > bridge panel, which is available on same PCB with 24-bit RGB interface.
> > > >
> > > > So, this patch adds DSI specific binding details on existing
> > > > dt-bindings file.
> > > >
> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > ---
> > > > Changes for v3:
> > > > - Use existing binding doc and update dsi details
> > > > Changes for v2:
> > > > - none
> > > >
> > > >  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
> > > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > > > index 35bc0c839f49..b7855dc7c66f 100644
> > > > --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > > > +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > > > @@ -1,12 +1,39 @@
> > > >  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
> > > >
> > > > +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
> > > > +
> > > > +Depending on the variant, the PCB attached to the panel module either
> > > > +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
> > > > +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
> > > > +itself
> > >
> > > As I understand this is display board, which contains 'pure' RGB panel
> > > S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
> > > These are separate devices, just connected by vendor to simplify its
> > > assembly. Why don't you create then bridge driver for ICN6211 and RGB
> > > panel driver for S070WV20-CT16 - it looks more generic.
> > > Then you can describe both in dts and voila.
> > > Creating drivers for every combo of devices (panel + bridge), just
> > > because some vendor sells them together seems incorrect - we have
> > > devicetree for it.
> >
> > Rob suggested this, and also the opposite: using the same
> > "bananapi,s070wv20-ct16"
> > compatible string for both types of connections, and have the driver deal with
> > detecting the bus type.
> >
> > The thing about the bridge chip is that there's no available datasheet that
> > describes all the parts of the init sequence, in fact none at all. I managed
> > to work out some bits, but the others remain a mystery and must be hard-coded
> > to match the panel. That would work against having a generic bridge driver.
>
> To me it seems logical that we'd model it as another step in the graph
> between the DSI component and the panel. It's conceivable that some
> other manufacturer will probably buy these for their panels and having
> a somewhat generic driver seems vaguely future proof to me.
>
> As I see it, the weird init process belongs to the bridge chip and the
> timings belong to the panel and we shouldn't "confuse" them by giving
> them the same compatible.

But the problem here is due to lack of information about the panel,
the init process command opcode data values seem to based on the panel
timings values. This look weird and ie reason for going into separate
panel driver with different compatible.

Re: [linux-sunxi] Re: [PATCH v3 17/25] dt-bindings: panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Julian Calaby]

Hi Jagan,

On Wed, Nov 7, 2018 at 5:13 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Wed, Oct 31, 2018 at 2:46 PM Julian Calaby <julian.calaby@gmail.com> wrote:
> >
> > Hi Jagan,
> >
> > On Wed, Oct 31, 2018 at 7:58 PM Chen-Yu Tsai <wens@csie.org> wrote:
> > >
> > > On Wed, Oct 31, 2018 at 4:53 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
> > > >
> > > > On 26.10.2018 16:43, Jagan Teki wrote:
> > > > > Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB
> > > > > bridge panel, which is available on same PCB with 24-bit RGB interface.
> > > > >
> > > > > So, this patch adds DSI specific binding details on existing
> > > > > dt-bindings file.
> > > > >
> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > ---
> > > > > Changes for v3:
> > > > > - Use existing binding doc and update dsi details
> > > > > Changes for v2:
> > > > > - none
> > > > >
> > > > >  .../display/panel/bananapi,s070wv20-ct16.txt  | 31 +++++++++++++++++--
> > > > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > > > > index 35bc0c839f49..b7855dc7c66f 100644
> > > > > --- a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > > > > +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> > > > > @@ -1,12 +1,39 @@
> > > > >  Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
> > > > >
> > > > > +S070WV20-CT16 is 7" 800x480 panel connected through a 24-bit RGB interface.
> > > > > +
> > > > > +Depending on the variant, the PCB attached to the panel module either
> > > > > +supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
> > > > > +an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
> > > > > +itself
> > > >
> > > > As I understand this is display board, which contains 'pure' RGB panel
> > > > S070WV20-CT16 and optionally ICN6211 DSI->RGB bridge.
> > > > These are separate devices, just connected by vendor to simplify its
> > > > assembly. Why don't you create then bridge driver for ICN6211 and RGB
> > > > panel driver for S070WV20-CT16 - it looks more generic.
> > > > Then you can describe both in dts and voila.
> > > > Creating drivers for every combo of devices (panel + bridge), just
> > > > because some vendor sells them together seems incorrect - we have
> > > > devicetree for it.
> > >
> > > Rob suggested this, and also the opposite: using the same
> > > "bananapi,s070wv20-ct16"
> > > compatible string for both types of connections, and have the driver deal with
> > > detecting the bus type.
> > >
> > > The thing about the bridge chip is that there's no available datasheet that
> > > describes all the parts of the init sequence, in fact none at all. I managed
> > > to work out some bits, but the others remain a mystery and must be hard-coded
> > > to match the panel. That would work against having a generic bridge driver.
> >
> > To me it seems logical that we'd model it as another step in the graph
> > between the DSI component and the panel. It's conceivable that some
> > other manufacturer will probably buy these for their panels and having
> > a somewhat generic driver seems vaguely future proof to me.
> >
> > As I see it, the weird init process belongs to the bridge chip and the
> > timings belong to the panel and we shouldn't "confuse" them by giving
> > them the same compatible.
>
> But the problem here is due to lack of information about the panel,
> the init process command opcode data values seem to based on the panel
> timings values. This look weird and ie reason for going into separate
> panel driver with different compatible.

I'd missed that particular wrinkle.

In that case, it makes sense to produce a panel driver with the bridge
chip's compatible.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

[PATCH v3 18/25] drm/panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Jagan Teki]

Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB bridge
panel which can be used to connect via DSI port on BPI-M64 board,
so add a driver for it.

The same panel PCB comes with parallel RBG which is supported via
panel-simple driver with "bananapi,s070wv20-ct16" compatible.

BSP dclock of 30MHz is not working with existing sunxi-ng and sun4i
sun4i_dclk_recalc, so updated to 55MHz can result proper working
nkm dividers.

dclock, 30MHz => PLL_MIPI, 300MHz => set rate 440MHz with 1,2,1 nkm
dclock, 55MHz => PLL_MIPI, 300MHz => set rate 330MHz with 5,2,9 nkm

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- Fixed to add proper commit message
- add tested credit
Changes for v2:
- none

 drivers/gpu/drm/panel/Kconfig                 |   9 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../panel/panel-bananapi-s070wv20-icn6211.c   | 336 ++++++++++++++++++
 3 files changed, 346 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-bananapi-s070wv20-icn6211.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 6020c30a33b3..20b88c275421 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -17,6 +17,15 @@ config DRM_PANEL_ARM_VERSATILE
 	  reference designs. The panel is detected using special registers
 	  in the Versatile family syscon registers.
 
+config DRM_PANEL_BANANAPI_S070WV20_ICN6211
+	tristate "Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge panel driver"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y if you want to enable support for panels based on the
+	  Bananapi S070WV20-CT16 MIPI-DSI controller.
+
 config DRM_PANEL_LVDS
 	tristate "Generic LVDS panel driver"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 5ccaaa9d13af..04696bb85218 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
+obj-$(CONFIG_DRM_PANEL_BANANAPI_S070WV20_ICN6211) += panel-bananapi-s070wv20-icn6211.o
 obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
diff --git a/drivers/gpu/drm/panel/panel-bananapi-s070wv20-icn6211.c b/drivers/gpu/drm/panel/panel-bananapi-s070wv20-icn6211.c
new file mode 100644
index 000000000000..643b215bec34
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-bananapi-s070wv20-icn6211.c
@@ -0,0 +1,336 @@
+// SPDX-License-Identifier: (GPL-2.0+ or MIT)
+/*
+ * Copyright (C) 2018 Amarula Solutions
+ * Author: Jagan Teki <jagan@amarulasolutions.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/fb.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+#include <video/mipi_display.h>
+
+struct s070wv20 {
+	struct drm_panel	panel;
+	struct mipi_dsi_device	*dsi;
+
+	struct backlight_device	*backlight;
+	struct regulator	*dvdd;
+	struct regulator	*avdd;
+	struct gpio_desc	*reset;
+
+	bool			is_enabled;
+	bool			is_prepared;
+};
+
+static inline struct s070wv20 *panel_to_s070wv20(struct drm_panel *panel)
+{
+	return container_of(panel, struct s070wv20, panel);
+}
+
+struct s070wv20_init_cmd {
+	size_t len;
+	const char *data;
+};
+
+#define S070WV20_INIT_CMD(...) { \
+	.len = sizeof((char[]){__VA_ARGS__}), \
+	.data = (char[]){__VA_ARGS__} }
+
+static const struct s070wv20_init_cmd s070wv20_init_cmds[] = {
+	S070WV20_INIT_CMD(0x7A, 0xC1),
+	S070WV20_INIT_CMD(0x20, 0x20),
+	S070WV20_INIT_CMD(0x21, 0xE0),
+	S070WV20_INIT_CMD(0x22, 0x13),
+	S070WV20_INIT_CMD(0x23, 0x28),
+	S070WV20_INIT_CMD(0x24, 0x30),
+	S070WV20_INIT_CMD(0x25, 0x28),
+	S070WV20_INIT_CMD(0x26, 0x00),
+	S070WV20_INIT_CMD(0x27, 0x0D),
+	S070WV20_INIT_CMD(0x28, 0x03),
+	S070WV20_INIT_CMD(0x29, 0x1D),
+	S070WV20_INIT_CMD(0x34, 0x80),
+	S070WV20_INIT_CMD(0x36, 0x28),
+	S070WV20_INIT_CMD(0xB5, 0xA0),
+	S070WV20_INIT_CMD(0x5C, 0xFF),
+	S070WV20_INIT_CMD(0x2A, 0x01),
+	S070WV20_INIT_CMD(0x56, 0x92),
+	S070WV20_INIT_CMD(0x6B, 0x71),
+	S070WV20_INIT_CMD(0x69, 0x2B),
+	S070WV20_INIT_CMD(0x10, 0x40),
+	S070WV20_INIT_CMD(0x11, 0x98),
+	S070WV20_INIT_CMD(0xB6, 0x20),
+	S070WV20_INIT_CMD(0x51, 0x20),
+	S070WV20_INIT_CMD(0x09, 0x10),
+};
+
+static int s070wv20_prepare(struct drm_panel *panel)
+{
+	struct s070wv20 *ctx = panel_to_s070wv20(panel);
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	unsigned int i;
+	int ret;
+
+	if (ctx->is_prepared)
+		return 0;
+
+	msleep(50);
+
+	gpiod_set_value(ctx->reset, 1);
+	msleep(50);
+
+	gpiod_set_value(ctx->reset, 0);
+	msleep(50);
+
+	gpiod_set_value(ctx->reset, 1);
+	msleep(20);
+
+	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(panel->dev, "failed to exit sleep mode: %d\n", ret);
+		return ret;
+	}
+
+	msleep(120);
+
+	for (i = 0; i < ARRAY_SIZE(s070wv20_init_cmds); i++) {
+		const struct s070wv20_init_cmd *cmd = &s070wv20_init_cmds[i];
+
+		ret = mipi_dsi_generic_write(dsi, cmd->data, cmd->len);
+		if (ret < 0)
+			return ret;
+
+		msleep(10);
+	}
+
+	ret = mipi_dsi_dcs_set_display_on(dsi);
+	if (ret < 0) {
+		dev_err(panel->dev, "failed to set display on: %d\n", ret);
+		return ret;
+	}
+
+	ctx->is_prepared = true;
+
+	return 0;
+}
+
+static int s070wv20_enable(struct drm_panel *panel)
+{
+	struct s070wv20 *ctx = panel_to_s070wv20(panel);
+
+	if (ctx->is_enabled)
+		return 0;
+
+	msleep(120);
+
+	backlight_enable(ctx->backlight);
+	ctx->is_enabled = true;
+
+	return 0;
+}
+
+static int s070wv20_disable(struct drm_panel *panel)
+{
+	struct s070wv20 *ctx = panel_to_s070wv20(panel);
+
+	if (!ctx->is_enabled)
+		return 0;
+
+	backlight_disable(ctx->backlight);
+	ctx->is_enabled = false;
+
+	return 0;
+}
+
+static int s070wv20_unprepare(struct drm_panel *panel)
+{
+	struct s070wv20 *ctx = panel_to_s070wv20(panel);
+	int ret;
+
+	if (!ctx->is_prepared)
+		return 0;
+
+	ret = mipi_dsi_dcs_set_display_off(ctx->dsi);
+	if (ret < 0)
+		dev_err(panel->dev, "failed to set display off: %d\n", ret);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+	if (ret < 0)
+		dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret);
+
+	msleep(100);
+
+	regulator_disable(ctx->avdd);
+
+	regulator_disable(ctx->dvdd);
+
+	gpiod_set_value(ctx->reset, 0);
+
+	gpiod_set_value(ctx->reset, 1);
+
+	gpiod_set_value(ctx->reset, 0);
+
+	ctx->is_prepared = false;
+
+	return 0;
+}
+
+static const struct drm_display_mode s070wv20_default_mode = {
+	.clock = 55000,
+	.vrefresh = 60,
+
+	.hdisplay = 800,
+	.hsync_start = 800 + 40,
+	.hsync_end = 800 + 40 + 48,
+	.htotal = 800 + 40 + 48 + 40,
+
+	.vdisplay = 480,
+	.vsync_start = 480 + 13,
+	.vsync_end = 480 + 13 + 3,
+	.vtotal = 480 + 13 + 3 + 29,
+};
+
+static int s070wv20_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct s070wv20 *ctx = panel_to_s070wv20(panel);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(panel->drm, &s070wv20_default_mode);
+	if (!mode) {
+		dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n",
+			s070wv20_default_mode.hdisplay,
+			s070wv20_default_mode.vdisplay,
+			s070wv20_default_mode.vrefresh);
+		return -ENOMEM;
+	}
+
+	drm_mode_set_name(mode);
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	panel->connector->display_info.width_mm = 86;
+	panel->connector->display_info.height_mm = 154;
+
+	return 1;
+}
+
+static const struct drm_panel_funcs s070wv20_funcs = {
+	.disable = s070wv20_disable,
+	.unprepare = s070wv20_unprepare,
+	.prepare = s070wv20_prepare,
+	.enable = s070wv20_enable,
+	.get_modes = s070wv20_get_modes,
+};
+
+static int s070wv20_dsi_probe(struct mipi_dsi_device *dsi)
+{
+	struct device_node *np;
+	struct s070wv20 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	mipi_dsi_set_drvdata(dsi, ctx);
+	ctx->dsi = dsi;
+
+	drm_panel_init(&ctx->panel);
+	ctx->panel.dev = &dsi->dev;
+	ctx->panel.funcs = &s070wv20_funcs;
+
+	ctx->dvdd = devm_regulator_get(&dsi->dev, "dvdd");
+	if (IS_ERR(ctx->dvdd)) {
+		dev_err(&dsi->dev, "Couldn't get dvdd regulator\n");
+		return PTR_ERR(ctx->dvdd);
+	}
+
+	ctx->avdd = devm_regulator_get(&dsi->dev, "avdd");
+	if (IS_ERR(ctx->avdd)) {
+		dev_err(&dsi->dev, "Couldn't get avdd regulator\n");
+		return PTR_ERR(ctx->avdd);
+	}
+
+	ret = regulator_enable(ctx->dvdd);
+	if (ret)
+		return ret;
+
+	msleep(5);
+
+	ret = regulator_enable(ctx->avdd);
+	if (ret)
+		return ret;
+
+	msleep(5);
+
+	ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->reset)) {
+		dev_err(&dsi->dev, "Couldn't get our reset GPIO\n");
+		return PTR_ERR(ctx->reset);
+	}
+
+	np = of_parse_phandle(dsi->dev.of_node, "backlight", 0);
+	if (np) {
+		ctx->backlight = of_find_backlight_by_node(np);
+		of_node_put(np);
+
+		if (!ctx->backlight)
+			return -EPROBE_DEFER;
+	}
+
+	ret = drm_panel_add(&ctx->panel);
+	if (ret < 0)
+		return ret;
+
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->lanes = 4;
+
+	return mipi_dsi_attach(dsi);
+}
+
+static int s070wv20_dsi_remove(struct mipi_dsi_device *dsi)
+{
+	struct s070wv20 *ctx = mipi_dsi_get_drvdata(dsi);
+
+	mipi_dsi_detach(dsi);
+	drm_panel_remove(&ctx->panel);
+
+	if (ctx->backlight)
+		put_device(&ctx->backlight->dev);
+
+	return 0;
+}
+
+static const struct of_device_id s070wv20_of_match[] = {
+	{ .compatible = "bananapi,s070wv20-ct16-icn6211", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, s070wv20_of_match);
+
+static struct mipi_dsi_driver s070wv20_driver = {
+	.probe = s070wv20_dsi_probe,
+	.remove = s070wv20_dsi_remove,
+	.driver = {
+		.name = "bananapi-s070wv20-ct16-icn6211",
+		.of_match_table = s070wv20_of_match,
+	},
+};
+module_mipi_dsi_driver(s070wv20_driver);
+
+MODULE_AUTHOR("Jagan Teki <jagan@amarulasolutions.com>");
+MODULE_DESCRIPTION("Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB");
+MODULE_LICENSE("GPL v2");
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v3 18/25] drm/panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

On Fri, Oct 26, 2018 at 08:13:37PM +0530, Jagan Teki wrote:
> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB bridge
> panel which can be used to connect via DSI port on BPI-M64 board,
> so add a driver for it.
> 
> The same panel PCB comes with parallel RBG which is supported via
> panel-simple driver with "bananapi,s070wv20-ct16" compatible.
> 
> BSP dclock of 30MHz is not working with existing sunxi-ng and sun4i
> sun4i_dclk_recalc, so updated to 55MHz can result proper working
> nkm dividers.
> 
> dclock, 30MHz => PLL_MIPI, 300MHz => set rate 440MHz with 1,2,1 nkm
> dclock, 55MHz => PLL_MIPI, 300MHz => set rate 330MHz with 5,2,9 nkm

That panel is generic, you don't need those details in your commit log.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v3 19/25] dt-bindings: panel: Add Techstar TS8550B MIPI DSI panel

[Jagan Teki]

Techstar TS8550B MIPI DSI panel is 480x854, 2-lane MIPI DSI
LCD panel. Add dt-bingings for it.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- new patch
Changes for v2:
- none

 .../display/panel/techstar,ts8550b.txt        | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/techstar,ts8550b.txt

diff --git a/Documentation/devicetree/bindings/display/panel/techstar,ts8550b.txt b/Documentation/devicetree/bindings/display/panel/techstar,ts8550b.txt
new file mode 100644
index 000000000000..6da050621d07
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/techstar,ts8550b.txt
@@ -0,0 +1,20 @@
+Techstar TS8550B MIPI DSI LCD Panel
+
+Required properties:
+- compatible: must be "techstar,ts8550b"
+- reg: DSI virtual channel used by that screen
+- avdd-supply: analog regulator dc1 switch
+- dvdd-supply: 3v3 digital regulator
+- reset-gpios: a GPIO phandle for the reset pin
+
+Optional properties:
+- backlight: phandle for the backlight control.
+
+panel@0 {
+	compatible = "techstar,ts8550b";
+	reg = <0>;
+	avdd-supply = <&reg_dc1sw>;
+	dvdd-supply = <&reg_dldo2>;
+	reset-gpios = <&pio 3 24 GPIO_ACTIVE_HIGH>; /* PD24 */
+	backlight = <&backlight>;
+};
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v3 19/25] dt-bindings: panel: Add Techstar TS8550B MIPI DSI panel

[Rob Herring]

On Fri, 26 Oct 2018 20:13:38 +0530, Jagan Teki wrote:
> Techstar TS8550B MIPI DSI panel is 480x854, 2-lane MIPI DSI
> LCD panel. Add dt-bingings for it.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - new patch
> Changes for v2:
> - none
> 
>  .../display/panel/techstar,ts8550b.txt        | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/techstar,ts8550b.txt
> 

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

[PATCH v3 20/25] drm/panel: Add Techstar TS8550B MIPI-DSI LCD panel

[Jagan Teki]

Techstar TS8550B MIPI DSI panel is 480x854, 2-lane MIPI DSI
LCD panel. Add panel driver for it.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- new patch
Changes for v2:
- none

 drivers/gpu/drm/panel/Kconfig                 |   9 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-techstar-ts8550b.c    | 346 ++++++++++++++++++
 3 files changed, 356 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-techstar-ts8550b.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 20b88c275421..d0d4e60f5153 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -195,4 +195,13 @@ config DRM_PANEL_SITRONIX_ST7789V
 	  Say Y here if you want to enable support for the Sitronix
 	  ST7789V controller for 240x320 LCD panels
 
+config DRM_PANEL_TECHSTAR_TS8550B
+	tristate "Techstar TS8550B MIPI-DSI panel driver"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y if you want to enable support for panels based on the
+	  Techstar TS8550B MIPI-DSI interface.
+
 endmenu
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 04696bb85218..88011f06edb8 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
 obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
+obj-$(CONFIG_DRM_PANEL_TECHSTAR_TS8550B) += panel-techstar-ts8550b.o
diff --git a/drivers/gpu/drm/panel/panel-techstar-ts8550b.c b/drivers/gpu/drm/panel/panel-techstar-ts8550b.c
new file mode 100644
index 000000000000..8baca71595a7
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-techstar-ts8550b.c
@@ -0,0 +1,346 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019, Amarula Solutions.
+ * Author: Jagan Teki <jagan@amarulasolutions.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/fb.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+#include <video/mipi_display.h>
+
+struct ts8550b {
+	struct drm_panel	panel;
+	struct mipi_dsi_device	*dsi;
+
+	struct backlight_device *backlight;
+	struct regulator	*dvdd;
+	struct regulator	*avdd;
+	struct gpio_desc	*reset;
+
+	bool			is_enabled;
+	bool			is_prepared;
+};
+
+static inline struct ts8550b *panel_to_ts8550b(struct drm_panel *panel)
+{
+	return container_of(panel, struct ts8550b, panel);
+}
+
+static inline int ts8550b_dcs_write_seq(struct ts8550b *ctx, const void *seq,
+					size_t len)
+{
+	return mipi_dsi_dcs_write_buffer(ctx->dsi, seq, len);
+};
+
+#define ts8550b_dcs_write_seq_static(ctx, seq...)		\
+	({							\
+		static const u8 d[] = { seq };			\
+		ts8550b_dcs_write_seq(ctx, d, ARRAY_SIZE(d));	\
+	})
+
+static void ts8550b_init_sequence(struct ts8550b *ctx)
+{
+	ts8550b_dcs_write_seq_static(ctx, 0x01, 0x00);
+	msleep(200);
+	ts8550b_dcs_write_seq_static(ctx, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x11);
+	ts8550b_dcs_write_seq_static(ctx, 0xD1, 0x11);
+	ts8550b_dcs_write_seq_static(ctx, 0x11, 0x00);
+	msleep(200);
+	ts8550b_dcs_write_seq_static(ctx, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x10);
+	ts8550b_dcs_write_seq_static(ctx, 0xC0, 0xE9, 0x03);
+	ts8550b_dcs_write_seq_static(ctx, 0xC1, 0x12, 0x02);
+	ts8550b_dcs_write_seq_static(ctx, 0xC2, 0x07, 0x06);
+	ts8550b_dcs_write_seq_static(ctx, 0xB0, 0x00, 0x0E, 0x15, 0x0F, 0x11,
+				     0x08, 0x08, 0x08, 0x08, 0x23, 0x04, 0x13,
+				     0x12, 0x2B, 0x34, 0x1F);
+	ts8550b_dcs_write_seq_static(ctx, 0xB1, 0x00, 0x0E, 0x95, 0x0F, 0x13,
+				     0x07, 0x09, 0x08, 0x08, 0x22, 0x04, 0x10,
+				     0x0E, 0x2C, 0x34, 0x1F);
+	ts8550b_dcs_write_seq_static(ctx, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x11);
+	ts8550b_dcs_write_seq_static(ctx, 0xB0, 0x45);
+	ts8550b_dcs_write_seq_static(ctx, 0xB1, 0x13);
+	ts8550b_dcs_write_seq_static(ctx, 0xB2, 0x07);
+	ts8550b_dcs_write_seq_static(ctx, 0xB3, 0x80);
+	ts8550b_dcs_write_seq_static(ctx, 0xB5, 0x47);
+	ts8550b_dcs_write_seq_static(ctx, 0xB7, 0x85);
+	ts8550b_dcs_write_seq_static(ctx, 0xB8, 0x20);
+	ts8550b_dcs_write_seq_static(ctx, 0xB9, 0x11);
+	ts8550b_dcs_write_seq_static(ctx, 0xC1, 0x78);
+	ts8550b_dcs_write_seq_static(ctx, 0xC2, 0x78);
+	ts8550b_dcs_write_seq_static(ctx, 0xD0, 0x88);
+	msleep(100);
+	ts8550b_dcs_write_seq_static(ctx, 0xE0, 0x00, 0x00, 0x02);
+	ts8550b_dcs_write_seq_static(ctx, 0xE1, 0x0B, 0x00, 0x0D, 0x00, 0x0C,
+				     0x00, 0x0E, 0x00, 0x00, 0x44, 0x44);
+	ts8550b_dcs_write_seq_static(ctx, 0xE2, 0x33, 0x33, 0x44, 0x44, 0x64,
+				     0x00, 0x66, 0x00, 0x65, 0x00, 0x67, 0x00,
+				     0x00);
+	ts8550b_dcs_write_seq_static(ctx, 0xE3, 0x00, 0x00, 0x33, 0x33);
+	ts8550b_dcs_write_seq_static(ctx, 0xE4, 0x44, 0x44);
+	ts8550b_dcs_write_seq_static(ctx, 0xE5, 0x0C, 0x78, 0x3C, 0xA0, 0x0E,
+				     0x78, 0x3C, 0xA0, 0x10, 0x78, 0x3C, 0xA0,
+				     0x12, 0x78, 0x3C, 0xA0);
+	ts8550b_dcs_write_seq_static(ctx, 0xE6, 0x00, 0x00, 0x33, 0x33);
+	ts8550b_dcs_write_seq_static(ctx, 0xE7, 0x44, 0x44);
+	ts8550b_dcs_write_seq_static(ctx, 0xE8, 0x0D, 0x78, 0x3C, 0xA0, 0x0F,
+				     0x78, 0x3C, 0xA0, 0x11, 0x78, 0x3C, 0xA0,
+				     0x13, 0x78, 0x3C, 0xA0);
+	ts8550b_dcs_write_seq_static(ctx, 0xEB, 0x02, 0x02, 0x39, 0x39, 0xEE,
+				     0x44, 0x00);
+	ts8550b_dcs_write_seq_static(ctx, 0xEC, 0x00, 0x00);
+	ts8550b_dcs_write_seq_static(ctx, 0xED, 0xFF, 0xF1, 0x04, 0x56, 0x72,
+				     0x3F, 0xFF, 0xFF, 0xFF, 0xFF, 0xF3, 0x27,
+				     0x65, 0x40, 0x1F, 0xFF);
+	ts8550b_dcs_write_seq_static(ctx, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x00);
+	msleep(10);
+	ts8550b_dcs_write_seq_static(ctx, 0x29, 0x00);
+	msleep(200);
+}
+
+static int ts8550b_prepare(struct drm_panel *panel)
+{
+	struct ts8550b *ctx = panel_to_ts8550b(panel);
+	int ret;
+
+	if (ctx->is_prepared)
+		return 0;
+
+	gpiod_set_value(ctx->reset, 0);
+	msleep(20);
+
+	ret = regulator_enable(ctx->dvdd);
+	if (ret)
+		return ret;
+	msleep(20);
+
+	ret = regulator_enable(ctx->avdd);
+	if (ret)
+		return ret;
+	msleep(20);
+
+	gpiod_set_value(ctx->reset, 1);
+	msleep(20);
+
+	gpiod_set_value(ctx->reset, 0);
+	msleep(30);
+
+	gpiod_set_value(ctx->reset, 1);
+	msleep(150);
+
+	ts8550b_init_sequence(ctx);
+
+	ctx->is_prepared = true;
+
+	return 0;
+}
+
+static int ts8550b_enable(struct drm_panel *panel)
+{
+	struct ts8550b *ctx = panel_to_ts8550b(panel);
+
+	if (ctx->is_enabled)
+		return 0;
+
+	msleep(120);
+
+	backlight_enable(ctx->backlight);
+	ctx->is_enabled = true;
+
+	return 0;
+}
+
+static int ts8550b_disable(struct drm_panel *panel)
+{
+	struct ts8550b *ctx = panel_to_ts8550b(panel);
+
+	if (!ctx->is_enabled)
+		return 0;
+
+	backlight_disable(ctx->backlight);
+	ctx->is_enabled = false;
+
+	return 0;
+}
+
+static int ts8550b_unprepare(struct drm_panel *panel)
+{
+	struct ts8550b *ctx = panel_to_ts8550b(panel);
+	int ret;
+
+	if (!ctx->is_prepared)
+		return 0;
+
+	ret = mipi_dsi_dcs_set_display_off(ctx->dsi);
+	if (ret < 0)
+		dev_err(panel->dev, "failed to set display off: %d\n", ret);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+	if (ret < 0)
+		dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret);
+
+	msleep(120);
+
+	regulator_disable(ctx->dvdd);
+
+	regulator_disable(ctx->avdd);
+
+	gpiod_set_value(ctx->reset, 0);
+
+	gpiod_set_value(ctx->reset, 1);
+
+	gpiod_set_value(ctx->reset, 0);
+
+	ctx->is_prepared = false;
+
+	return 0;
+}
+
+static const struct drm_display_mode default_mode = {
+	.clock		= 27500,
+	.vrefresh	= 60,
+
+	.hdisplay	= 480,
+	.hsync_start	= 480 + 38,
+	.hsync_end	= 480 + 38 + 12,
+	.htotal		= 480 + 38 + 12 + 12,
+
+	.vdisplay	= 854,
+	.vsync_start	= 854 + 18,
+	.vsync_end	= 854 + 18 + 8,
+	.vtotal		= 854 + 18 + 8 + 4,
+};
+
+static int ts8550b_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct ts8550b *ctx = panel_to_ts8550b(panel);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(panel->drm, &default_mode);
+	if (!mode) {
+		dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n",
+			default_mode.hdisplay, default_mode.vdisplay,
+			default_mode.vrefresh);
+		return -ENOMEM;
+	}
+
+	drm_mode_set_name(mode);
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	panel->connector->display_info.width_mm = 69;
+	panel->connector->display_info.height_mm = 139;
+
+	return 1;
+}
+
+static const struct drm_panel_funcs ts8550b_funcs = {
+	.disable	= ts8550b_disable,
+	.unprepare	= ts8550b_unprepare,
+	.prepare	= ts8550b_prepare,
+	.enable		= ts8550b_enable,
+	.get_modes	= ts8550b_get_modes,
+};
+
+static int ts8550b_dsi_probe(struct mipi_dsi_device *dsi)
+{
+	struct device_node *np;
+	struct ts8550b *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	mipi_dsi_set_drvdata(dsi, ctx);
+	ctx->dsi = dsi;
+
+	drm_panel_init(&ctx->panel);
+	ctx->panel.dev = &dsi->dev;
+	ctx->panel.funcs = &ts8550b_funcs;
+
+	ctx->dvdd = devm_regulator_get(&dsi->dev, "dvdd");
+	if (IS_ERR(ctx->dvdd)) {
+		dev_err(&dsi->dev, "Couldn't get dvdd regulator\n");
+		return PTR_ERR(ctx->dvdd);
+	}
+
+	ctx->avdd = devm_regulator_get(&dsi->dev, "avdd");
+	if (IS_ERR(ctx->avdd)) {
+		dev_err(&dsi->dev, "Couldn't get avdd regulator\n");
+		return PTR_ERR(ctx->avdd);
+	}
+
+	ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->reset)) {
+		dev_err(&dsi->dev, "Couldn't get our reset GPIO\n");
+		return PTR_ERR(ctx->reset);
+	}
+
+	np = of_parse_phandle(dsi->dev.of_node, "backlight", 0);
+	if (np) {
+		ctx->backlight = of_find_backlight_by_node(np);
+		of_node_put(np);
+
+		if (!ctx->backlight)
+			return -EPROBE_DEFER;
+	}
+
+	ret = drm_panel_add(&ctx->panel);
+	if (ret < 0)
+		return ret;
+
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->lanes = 2;
+
+	return mipi_dsi_attach(dsi);
+}
+
+static int ts8550b_dsi_remove(struct mipi_dsi_device *dsi)
+{
+	struct ts8550b *ctx = mipi_dsi_get_drvdata(dsi);
+
+	mipi_dsi_detach(dsi);
+	drm_panel_remove(&ctx->panel);
+
+	if (ctx->backlight)
+		put_device(&ctx->backlight->dev);
+
+	return 0;
+}
+
+static const struct of_device_id ts8550b_of_match[] = {
+	{ .compatible = "techstar,ts8550b" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ts8550b_of_match);
+
+static struct mipi_dsi_driver ts8550b_dsi_driver = {
+	.probe		= ts8550b_dsi_probe,
+	.remove		= ts8550b_dsi_remove,
+	.driver = {
+		.name		= "techstar-ts8550b",
+		.of_match_table	= ts8550b_of_match,
+	},
+};
+module_mipi_dsi_driver(ts8550b_dsi_driver);
+
+MODULE_AUTHOR("Jagan Teki <jagan@amarulasolutions.com>");
+MODULE_DESCRIPTION("Techstar TS8550B MIPI-DSI LCD Panel Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.18.0.321.gffc6fa0e3

Re: [linux-sunxi] [PATCH v3 20/25] drm/panel: Add Techstar TS8550B MIPI-DSI LCD panel

[Priit Laes]

On Fri, Oct 26, 2018 at 08:13:39PM +0530, Jagan Teki wrote:
> Techstar TS8550B MIPI DSI panel is 480x854, 2-lane MIPI DSI
> LCD panel. Add panel driver for it.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - new patch
> Changes for v2:
> - none
> 
>  drivers/gpu/drm/panel/Kconfig                 |   9 +
>  drivers/gpu/drm/panel/Makefile                |   1 +
>  .../gpu/drm/panel/panel-techstar-ts8550b.c    | 346 ++++++++++++++++++
>  3 files changed, 356 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-techstar-ts8550b.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 20b88c275421..d0d4e60f5153 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -195,4 +195,13 @@ config DRM_PANEL_SITRONIX_ST7789V
>  	  Say Y here if you want to enable support for the Sitronix
>  	  ST7789V controller for 240x320 LCD panels
>  
> +config DRM_PANEL_TECHSTAR_TS8550B
> +	tristate "Techstar TS8550B MIPI-DSI panel driver"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y if you want to enable support for panels based on the
> +	  Techstar TS8550B MIPI-DSI interface.
> +
>  endmenu
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 04696bb85218..88011f06edb8 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -20,3 +20,4 @@ obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
>  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> +obj-$(CONFIG_DRM_PANEL_TECHSTAR_TS8550B) += panel-techstar-ts8550b.o
> diff --git a/drivers/gpu/drm/panel/panel-techstar-ts8550b.c b/drivers/gpu/drm/panel/panel-techstar-ts8550b.c
> new file mode 100644
> index 000000000000..8baca71595a7
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-techstar-ts8550b.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019, Amarula Solutions.
> + * Author: Jagan Teki <jagan@amarulasolutions.com>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/fb.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct ts8550b {
> +	struct drm_panel	panel;
> +	struct mipi_dsi_device	*dsi;
> +
> +	struct backlight_device *backlight;
> +	struct regulator	*dvdd;
> +	struct regulator	*avdd;
> +	struct gpio_desc	*reset;
> +
> +	bool			is_enabled;
> +	bool			is_prepared;
> +};
> +
> +static inline struct ts8550b *panel_to_ts8550b(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct ts8550b, panel);
> +}
> +
> +static inline int ts8550b_dcs_write_seq(struct ts8550b *ctx, const void *seq,
> +					size_t len)
> +{
> +	return mipi_dsi_dcs_write_buffer(ctx->dsi, seq, len);
> +};
> +
> +#define ts8550b_dcs_write_seq_static(ctx, seq...)		\
> +	({							\
> +		static const u8 d[] = { seq };			\
> +		ts8550b_dcs_write_seq(ctx, d, ARRAY_SIZE(d));	\
> +	})
> +
> +static void ts8550b_init_sequence(struct ts8550b *ctx)

This seems to be pretty "standard" MIPI-based display, utilizing
bunch of manufacturer-specific commands during init sequence.

Standard MIPI commands fall between 0x00..0xAF and are defined in <video/mipi.h>

> +{
> +	ts8550b_dcs_write_seq_static(ctx, 0x01, 0x00);


MIPI_DCS_SOFT_RESET

> +	msleep(200);
> +	ts8550b_dcs_write_seq_static(ctx, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x11);
> +	ts8550b_dcs_write_seq_static(ctx, 0xD1, 0x11);
> +	ts8550b_dcs_write_seq_static(ctx, 0x11, 0x00);

MIPI_DCS_EXIT_SLEEP_MODE

> +	msleep(200);
> +	ts8550b_dcs_write_seq_static(ctx, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x10);
> +	ts8550b_dcs_write_seq_static(ctx, 0xC0, 0xE9, 0x03);
> +	ts8550b_dcs_write_seq_static(ctx, 0xC1, 0x12, 0x02);
> +	ts8550b_dcs_write_seq_static(ctx, 0xC2, 0x07, 0x06);
> +	ts8550b_dcs_write_seq_static(ctx, 0xB0, 0x00, 0x0E, 0x15, 0x0F, 0x11,
> +				     0x08, 0x08, 0x08, 0x08, 0x23, 0x04, 0x13,
> +				     0x12, 0x2B, 0x34, 0x1F);
> +	ts8550b_dcs_write_seq_static(ctx, 0xB1, 0x00, 0x0E, 0x95, 0x0F, 0x13,
> +				     0x07, 0x09, 0x08, 0x08, 0x22, 0x04, 0x10,
> +				     0x0E, 0x2C, 0x34, 0x1F);
> +	ts8550b_dcs_write_seq_static(ctx, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x11);
> +	ts8550b_dcs_write_seq_static(ctx, 0xB0, 0x45);
> +	ts8550b_dcs_write_seq_static(ctx, 0xB1, 0x13);
> +	ts8550b_dcs_write_seq_static(ctx, 0xB2, 0x07);
> +	ts8550b_dcs_write_seq_static(ctx, 0xB3, 0x80);
> +	ts8550b_dcs_write_seq_static(ctx, 0xB5, 0x47);
> +	ts8550b_dcs_write_seq_static(ctx, 0xB7, 0x85);
> +	ts8550b_dcs_write_seq_static(ctx, 0xB8, 0x20);
> +	ts8550b_dcs_write_seq_static(ctx, 0xB9, 0x11);
> +	ts8550b_dcs_write_seq_static(ctx, 0xC1, 0x78);
> +	ts8550b_dcs_write_seq_static(ctx, 0xC2, 0x78);
> +	ts8550b_dcs_write_seq_static(ctx, 0xD0, 0x88);
> +	msleep(100);
> +	ts8550b_dcs_write_seq_static(ctx, 0xE0, 0x00, 0x00, 0x02);
> +	ts8550b_dcs_write_seq_static(ctx, 0xE1, 0x0B, 0x00, 0x0D, 0x00, 0x0C,
> +				     0x00, 0x0E, 0x00, 0x00, 0x44, 0x44);
> +	ts8550b_dcs_write_seq_static(ctx, 0xE2, 0x33, 0x33, 0x44, 0x44, 0x64,
> +				     0x00, 0x66, 0x00, 0x65, 0x00, 0x67, 0x00,
> +				     0x00);
> +	ts8550b_dcs_write_seq_static(ctx, 0xE3, 0x00, 0x00, 0x33, 0x33);
> +	ts8550b_dcs_write_seq_static(ctx, 0xE4, 0x44, 0x44);
> +	ts8550b_dcs_write_seq_static(ctx, 0xE5, 0x0C, 0x78, 0x3C, 0xA0, 0x0E,
> +				     0x78, 0x3C, 0xA0, 0x10, 0x78, 0x3C, 0xA0,
> +				     0x12, 0x78, 0x3C, 0xA0);
> +	ts8550b_dcs_write_seq_static(ctx, 0xE6, 0x00, 0x00, 0x33, 0x33);
> +	ts8550b_dcs_write_seq_static(ctx, 0xE7, 0x44, 0x44);
> +	ts8550b_dcs_write_seq_static(ctx, 0xE8, 0x0D, 0x78, 0x3C, 0xA0, 0x0F,
> +				     0x78, 0x3C, 0xA0, 0x11, 0x78, 0x3C, 0xA0,
> +				     0x13, 0x78, 0x3C, 0xA0);
> +	ts8550b_dcs_write_seq_static(ctx, 0xEB, 0x02, 0x02, 0x39, 0x39, 0xEE,
> +				     0x44, 0x00);
> +	ts8550b_dcs_write_seq_static(ctx, 0xEC, 0x00, 0x00);
> +	ts8550b_dcs_write_seq_static(ctx, 0xED, 0xFF, 0xF1, 0x04, 0x56, 0x72,
> +				     0x3F, 0xFF, 0xFF, 0xFF, 0xFF, 0xF3, 0x27,
> +				     0x65, 0x40, 0x1F, 0xFF);
> +	ts8550b_dcs_write_seq_static(ctx, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x00);
> +	msleep(10);
> +	ts8550b_dcs_write_seq_static(ctx, 0x29, 0x00);

MIPI_DCS_SET_DISPLAY_ON

> +	msleep(200);
> +}
> +
> +static int ts8550b_prepare(struct drm_panel *panel)
> +{
> +	struct ts8550b *ctx = panel_to_ts8550b(panel);
> +	int ret;
> +
> +	if (ctx->is_prepared)
> +		return 0;
> +
> +	gpiod_set_value(ctx->reset, 0);
> +	msleep(20);
> +
> +	ret = regulator_enable(ctx->dvdd);
> +	if (ret)
> +		return ret;
> +	msleep(20);
> +
> +	ret = regulator_enable(ctx->avdd);
> +	if (ret)
> +		return ret;
> +	msleep(20);
> +
> +	gpiod_set_value(ctx->reset, 1);
> +	msleep(20);
> +
> +	gpiod_set_value(ctx->reset, 0);
> +	msleep(30);
> +
> +	gpiod_set_value(ctx->reset, 1);
> +	msleep(150);
> +
> +	ts8550b_init_sequence(ctx);
> +
> +	ctx->is_prepared = true;
> +
> +	return 0;
> +}
> +
> +static int ts8550b_enable(struct drm_panel *panel)
> +{
> +	struct ts8550b *ctx = panel_to_ts8550b(panel);
> +
> +	if (ctx->is_enabled)
> +		return 0;
> +
> +	msleep(120);
> +
> +	backlight_enable(ctx->backlight);
> +	ctx->is_enabled = true;
> +
> +	return 0;
> +}
> +
> +static int ts8550b_disable(struct drm_panel *panel)
> +{
> +	struct ts8550b *ctx = panel_to_ts8550b(panel);
> +
> +	if (!ctx->is_enabled)
> +		return 0;
> +
> +	backlight_disable(ctx->backlight);
> +	ctx->is_enabled = false;
> +
> +	return 0;
> +}
> +
> +static int ts8550b_unprepare(struct drm_panel *panel)
> +{
> +	struct ts8550b *ctx = panel_to_ts8550b(panel);
> +	int ret;
> +
> +	if (!ctx->is_prepared)
> +		return 0;
> +
> +	ret = mipi_dsi_dcs_set_display_off(ctx->dsi);
> +	if (ret < 0)
> +		dev_err(panel->dev, "failed to set display off: %d\n", ret);
> +
> +	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
> +	if (ret < 0)
> +		dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret);
> +
> +	msleep(120);
> +
> +	regulator_disable(ctx->dvdd);
> +
> +	regulator_disable(ctx->avdd);
> +
> +	gpiod_set_value(ctx->reset, 0);
> +
> +	gpiod_set_value(ctx->reset, 1);
> +
> +	gpiod_set_value(ctx->reset, 0);
> +
> +	ctx->is_prepared = false;
> +
> +	return 0;
> +}
> +
> +static const struct drm_display_mode default_mode = {
> +	.clock		= 27500,
> +	.vrefresh	= 60,
> +
> +	.hdisplay	= 480,
> +	.hsync_start	= 480 + 38,
> +	.hsync_end	= 480 + 38 + 12,
> +	.htotal		= 480 + 38 + 12 + 12,
> +
> +	.vdisplay	= 854,
> +	.vsync_start	= 854 + 18,
> +	.vsync_end	= 854 + 18 + 8,
> +	.vtotal		= 854 + 18 + 8 + 4,
> +};
> +
> +static int ts8550b_get_modes(struct drm_panel *panel)
> +{
> +	struct drm_connector *connector = panel->connector;
> +	struct ts8550b *ctx = panel_to_ts8550b(panel);
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(panel->drm, &default_mode);
> +	if (!mode) {
> +		dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n",
> +			default_mode.hdisplay, default_mode.vdisplay,
> +			default_mode.vrefresh);
> +		return -ENOMEM;
> +	}
> +
> +	drm_mode_set_name(mode);
> +
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_probed_add(connector, mode);
> +
> +	panel->connector->display_info.width_mm = 69;
> +	panel->connector->display_info.height_mm = 139;
> +
> +	return 1;
> +}
> +
> +static const struct drm_panel_funcs ts8550b_funcs = {
> +	.disable	= ts8550b_disable,
> +	.unprepare	= ts8550b_unprepare,
> +	.prepare	= ts8550b_prepare,
> +	.enable		= ts8550b_enable,
> +	.get_modes	= ts8550b_get_modes,
> +};
> +
> +static int ts8550b_dsi_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device_node *np;
> +	struct ts8550b *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +	ctx->dsi = dsi;
> +
> +	drm_panel_init(&ctx->panel);
> +	ctx->panel.dev = &dsi->dev;
> +	ctx->panel.funcs = &ts8550b_funcs;
> +
> +	ctx->dvdd = devm_regulator_get(&dsi->dev, "dvdd");
> +	if (IS_ERR(ctx->dvdd)) {
> +		dev_err(&dsi->dev, "Couldn't get dvdd regulator\n");
> +		return PTR_ERR(ctx->dvdd);
> +	}
> +
> +	ctx->avdd = devm_regulator_get(&dsi->dev, "avdd");
> +	if (IS_ERR(ctx->avdd)) {
> +		dev_err(&dsi->dev, "Couldn't get avdd regulator\n");
> +		return PTR_ERR(ctx->avdd);
> +	}
> +
> +	ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset)) {
> +		dev_err(&dsi->dev, "Couldn't get our reset GPIO\n");
> +		return PTR_ERR(ctx->reset);
> +	}
> +
> +	np = of_parse_phandle(dsi->dev.of_node, "backlight", 0);
> +	if (np) {
> +		ctx->backlight = of_find_backlight_by_node(np);
> +		of_node_put(np);
> +
> +		if (!ctx->backlight)
> +			return -EPROBE_DEFER;
> +	}
> +
> +	ret = drm_panel_add(&ctx->panel);
> +	if (ret < 0)
> +		return ret;
> +
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->lanes = 2;
> +
> +	return mipi_dsi_attach(dsi);
> +}
> +
> +static int ts8550b_dsi_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct ts8550b *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	mipi_dsi_detach(dsi);
> +	drm_panel_remove(&ctx->panel);
> +
> +	if (ctx->backlight)
> +		put_device(&ctx->backlight->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ts8550b_of_match[] = {
> +	{ .compatible = "techstar,ts8550b" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ts8550b_of_match);
> +
> +static struct mipi_dsi_driver ts8550b_dsi_driver = {
> +	.probe		= ts8550b_dsi_probe,
> +	.remove		= ts8550b_dsi_remove,
> +	.driver = {
> +		.name		= "techstar-ts8550b",
> +		.of_match_table	= ts8550b_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(ts8550b_dsi_driver);
> +
> +MODULE_AUTHOR("Jagan Teki <jagan@amarulasolutions.com>");
> +MODULE_DESCRIPTION("Techstar TS8550B MIPI-DSI LCD Panel Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.18.0.321.gffc6fa0e3
> 
> -- 
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] [PATCH v3 20/25] drm/panel: Add Techstar TS8550B MIPI-DSI LCD panel

[Jagan Teki]

On Fri, Oct 26, 2018 at 9:43 PM Priit Laes <plaes@plaes.org> wrote:
>
> On Fri, Oct 26, 2018 at 08:13:39PM +0530, Jagan Teki wrote:
> > Techstar TS8550B MIPI DSI panel is 480x854, 2-lane MIPI DSI
> > LCD panel. Add panel driver for it.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v3:
> > - new patch
> > Changes for v2:
> > - none
> >
> >  drivers/gpu/drm/panel/Kconfig                 |   9 +
> >  drivers/gpu/drm/panel/Makefile                |   1 +
> >  .../gpu/drm/panel/panel-techstar-ts8550b.c    | 346 ++++++++++++++++++
> >  3 files changed, 356 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panel/panel-techstar-ts8550b.c
> >
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index 20b88c275421..d0d4e60f5153 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -195,4 +195,13 @@ config DRM_PANEL_SITRONIX_ST7789V
> >         Say Y here if you want to enable support for the Sitronix
> >         ST7789V controller for 240x320 LCD panels
> >
> > +config DRM_PANEL_TECHSTAR_TS8550B
> > +     tristate "Techstar TS8550B MIPI-DSI panel driver"
> > +     depends on OF
> > +     depends on DRM_MIPI_DSI
> > +     depends on BACKLIGHT_CLASS_DEVICE
> > +     help
> > +       Say Y if you want to enable support for panels based on the
> > +       Techstar TS8550B MIPI-DSI interface.
> > +
> >  endmenu
> > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > index 04696bb85218..88011f06edb8 100644
> > --- a/drivers/gpu/drm/panel/Makefile
> > +++ b/drivers/gpu/drm/panel/Makefile
> > @@ -20,3 +20,4 @@ obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
> >  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
> >  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> >  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> > +obj-$(CONFIG_DRM_PANEL_TECHSTAR_TS8550B) += panel-techstar-ts8550b.o
> > diff --git a/drivers/gpu/drm/panel/panel-techstar-ts8550b.c b/drivers/gpu/drm/panel/panel-techstar-ts8550b.c
> > new file mode 100644
> > index 000000000000..8baca71595a7
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-techstar-ts8550b.c
> > @@ -0,0 +1,346 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2019, Amarula Solutions.
> > + * Author: Jagan Teki <jagan@amarulasolutions.com>
> > + */
> > +
> > +#include <linux/backlight.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/errno.h>
> > +#include <linux/fb.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +#include <drm/drm_mipi_dsi.h>
> > +#include <drm/drm_modes.h>
> > +#include <drm/drm_panel.h>
> > +
> > +#include <video/mipi_display.h>
> > +
> > +struct ts8550b {
> > +     struct drm_panel        panel;
> > +     struct mipi_dsi_device  *dsi;
> > +
> > +     struct backlight_device *backlight;
> > +     struct regulator        *dvdd;
> > +     struct regulator        *avdd;
> > +     struct gpio_desc        *reset;
> > +
> > +     bool                    is_enabled;
> > +     bool                    is_prepared;
> > +};
> > +
> > +static inline struct ts8550b *panel_to_ts8550b(struct drm_panel *panel)
> > +{
> > +     return container_of(panel, struct ts8550b, panel);
> > +}
> > +
> > +static inline int ts8550b_dcs_write_seq(struct ts8550b *ctx, const void *seq,
> > +                                     size_t len)
> > +{
> > +     return mipi_dsi_dcs_write_buffer(ctx->dsi, seq, len);
> > +};
> > +
> > +#define ts8550b_dcs_write_seq_static(ctx, seq...)            \
> > +     ({                                                      \
> > +             static const u8 d[] = { seq };                  \
> > +             ts8550b_dcs_write_seq(ctx, d, ARRAY_SIZE(d));   \
> > +     })
> > +
> > +static void ts8550b_init_sequence(struct ts8550b *ctx)
>
> This seems to be pretty "standard" MIPI-based display, utilizing
> bunch of manufacturer-specific commands during init sequence.
>
> Standard MIPI commands fall between 0x00..0xAF and are defined in <video/mipi.h>

Where is this file? we have include/video/mipi_display.h with
0x00..0xA8. Not all commands in sequence is listed in this, may be I
will update DCS commands macros and rest as-it-is. what do you think?

Re: [linux-sunxi] [PATCH v3 20/25] drm/panel: Add Techstar TS8550B MIPI-DSI LCD panel

[Priit Laes]

On Sat, Oct 27, 2018 at 03:25:29PM +0530, Jagan Teki wrote:
> On Fri, Oct 26, 2018 at 9:43 PM Priit Laes <plaes@plaes.org> wrote:
> >
> > On Fri, Oct 26, 2018 at 08:13:39PM +0530, Jagan Teki wrote:
> > > Techstar TS8550B MIPI DSI panel is 480x854, 2-lane MIPI DSI
> > > LCD panel. Add panel driver for it.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > Tested-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > > Changes for v3:
> > > - new patch
> > > Changes for v2:
> > > - none
> > >
> > >  drivers/gpu/drm/panel/Kconfig                 |   9 +
> > >  drivers/gpu/drm/panel/Makefile                |   1 +
> > >  .../gpu/drm/panel/panel-techstar-ts8550b.c    | 346 ++++++++++++++++++
> > >  3 files changed, 356 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/panel/panel-techstar-ts8550b.c
> > >
> > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > > index 20b88c275421..d0d4e60f5153 100644
> > > --- a/drivers/gpu/drm/panel/Kconfig
> > > +++ b/drivers/gpu/drm/panel/Kconfig
> > > @@ -195,4 +195,13 @@ config DRM_PANEL_SITRONIX_ST7789V
> > >         Say Y here if you want to enable support for the Sitronix
> > >         ST7789V controller for 240x320 LCD panels
> > >
> > > +config DRM_PANEL_TECHSTAR_TS8550B
> > > +     tristate "Techstar TS8550B MIPI-DSI panel driver"
> > > +     depends on OF
> > > +     depends on DRM_MIPI_DSI
> > > +     depends on BACKLIGHT_CLASS_DEVICE
> > > +     help
> > > +       Say Y if you want to enable support for panels based on the
> > > +       Techstar TS8550B MIPI-DSI interface.
> > > +
> > >  endmenu
> > > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > > index 04696bb85218..88011f06edb8 100644
> > > --- a/drivers/gpu/drm/panel/Makefile
> > > +++ b/drivers/gpu/drm/panel/Makefile
> > > @@ -20,3 +20,4 @@ obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
> > >  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
> > >  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> > >  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> > > +obj-$(CONFIG_DRM_PANEL_TECHSTAR_TS8550B) += panel-techstar-ts8550b.o
> > > diff --git a/drivers/gpu/drm/panel/panel-techstar-ts8550b.c b/drivers/gpu/drm/panel/panel-techstar-ts8550b.c
> > > new file mode 100644
> > > index 000000000000..8baca71595a7
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/panel/panel-techstar-ts8550b.c
> > > @@ -0,0 +1,346 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2019, Amarula Solutions.
> > > + * Author: Jagan Teki <jagan@amarulasolutions.com>
> > > + */
> > > +
> > > +#include <linux/backlight.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/device.h>
> > > +#include <linux/err.h>
> > > +#include <linux/errno.h>
> > > +#include <linux/fb.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/regulator/consumer.h>
> > > +
> > > +#include <drm/drm_mipi_dsi.h>
> > > +#include <drm/drm_modes.h>
> > > +#include <drm/drm_panel.h>
> > > +
> > > +#include <video/mipi_display.h>
> > > +
> > > +struct ts8550b {
> > > +     struct drm_panel        panel;
> > > +     struct mipi_dsi_device  *dsi;
> > > +
> > > +     struct backlight_device *backlight;
> > > +     struct regulator        *dvdd;
> > > +     struct regulator        *avdd;
> > > +     struct gpio_desc        *reset;
> > > +
> > > +     bool                    is_enabled;
> > > +     bool                    is_prepared;
> > > +};
> > > +
> > > +static inline struct ts8550b *panel_to_ts8550b(struct drm_panel *panel)
> > > +{
> > > +     return container_of(panel, struct ts8550b, panel);
> > > +}
> > > +
> > > +static inline int ts8550b_dcs_write_seq(struct ts8550b *ctx, const void *seq,
> > > +                                     size_t len)
> > > +{
> > > +     return mipi_dsi_dcs_write_buffer(ctx->dsi, seq, len);
> > > +};
> > > +
> > > +#define ts8550b_dcs_write_seq_static(ctx, seq...)            \
> > > +     ({                                                      \
> > > +             static const u8 d[] = { seq };                  \
> > > +             ts8550b_dcs_write_seq(ctx, d, ARRAY_SIZE(d));   \
> > > +     })
> > > +
> > > +static void ts8550b_init_sequence(struct ts8550b *ctx)
> >
> > This seems to be pretty "standard" MIPI-based display, utilizing
> > bunch of manufacturer-specific commands during init sequence.
> >
> > Standard MIPI commands fall between 0x00..0xAF and are defined in <video/mipi.h>
> 
> Where is this file? we have include/video/mipi_display.h with
> 0x00..0xA8. Not all commands in sequence is listed in this, may be I
> will update DCS commands macros and rest as-it-is. what do you think?

Yeah, include/video/mipi_display.h is the header file I was referring to.

Although those three commands I pointed out look like standard DCS command
defines and the order makes sense too:
SOFT_RESET -> magic1 -> EXIT_SLEEP_MODE -> magic2 -> SET_DISPLAY_ON

Until someone finds proper datasheet for this device, those magic1
and magic2 sequenses remain mystery...

Päikest,
Priit :)

[PATCH v3 21/25] clk: sunxi-ng: a64: Add min and max rate for PLL_MIPI

[Jagan Teki]

Manual noted to use PLL_MIPI rate 500MHz to 1.4GHz,
but lowering the min rate by 300MHz can result proper
working nkms divider with the help of desired dclock
rate from panel driver.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Acked-by: Stephen Boyd <sboyd@kernel.org>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- collect Stephen Ack
- add tested credit
Changes for v2:
- none

 drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 019d67bf97c4..5a3a5b821f8b 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -167,6 +167,8 @@ static struct ccu_nkm pll_mipi_clk = {
 	.n		= _SUNXI_CCU_MULT(8, 4),
 	.k		= _SUNXI_CCU_MULT_MIN(4, 2, 2),
 	.m		= _SUNXI_CCU_DIV(0, 4),
+	.min_rate	= 300000000,		/* Actual rate is 500MHz */
+	.max_rate	= 1400000000UL,
 	.common		= {
 		.reg		= 0x040,
 		.hw.init	= CLK_HW_INIT("pll-mipi", "pll-video0",
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 22/25] dt-bindings: sun6i-dsi: Add compatible for A64 DPHY

[Jagan Teki]

The MIPI DSI PHY HDMI controller on Allwinner A64 is similar
on the one on A31.

Add A64 compatible and append A31 compatible as fallback.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- none
Changes for v2:
- new patch

 Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt
index 9fa6e7a758ad..704fb31962f2 100644
--- a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt
+++ b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt
@@ -36,7 +36,7 @@ D-PHY
 
 Required properties:
   - compatible: value must be one of:
-    * allwinner,sun6i-a31-mipi-dphy
+    * "allwinner,sun50i-a64-mipi-dphy", "allwinner,sun6i-a31-mipi-dphy"
   - reg: base address and size of memory-mapped region
   - clocks: phandles to the clocks feeding the DSI encoder
     * bus: the DSI interface clock
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v3 22/25] dt-bindings: sun6i-dsi: Add compatible for A64 DPHY

[Rob Herring]

On Fri, 26 Oct 2018 20:13:41 +0530, Jagan Teki wrote:
> The MIPI DSI PHY HDMI controller on Allwinner A64 is similar
> on the one on A31.
> 
> Add A64 compatible and append A31 compatible as fallback.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - none
> Changes for v2:
> - new patch
> 
>  Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

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

Re: [PATCH v3 22/25] dt-bindings: sun6i-dsi: Add compatible for A64 DPHY

[Chen-Yu Tsai]

On Fri, Oct 26, 2018 at 10:46 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> The MIPI DSI PHY HDMI controller on Allwinner A64 is similar
> on the one on A31.
>
> Add A64 compatible and append A31 compatible as fallback.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - none
> Changes for v2:
> - new patch
>
>  Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt
> index 9fa6e7a758ad..704fb31962f2 100644
> --- a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt
> @@ -36,7 +36,7 @@ D-PHY
>
>  Required properties:
>    - compatible: value must be one of:
> -    * allwinner,sun6i-a31-mipi-dphy
> +    * "allwinner,sun50i-a64-mipi-dphy", "allwinner,sun6i-a31-mipi-dphy"

The format for this is one "combination" per line. You need to keep the
original "allwinner,sun6i-a31-mipi-dphy".

ChenYu

>    - reg: base address and size of memory-mapped region
>    - clocks: phandles to the clocks feeding the DSI encoder
>      * bus: the DSI interface clock
> --
> 2.18.0.321.gffc6fa0e3
>

[PATCH v3 23/25] arm64: dts: allwinner: a64: Add DSI pipeline

[Jagan Teki]

The A64 has a MIPI-DSI block which is similar to A31
without mod clock.

So, add dsi node with A64 compatible, dphy node with
A31 compatible and finally connect dsi to tcon0 to
make proper DSI pipeline.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- add allwinner,tcon-channel property
- add dsi nodes in ascending physical address 
Changes for v2:
- use A64 dphy compatible

 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index f3a66f888205..d6f269883759 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -320,6 +320,12 @@
 					#address-cells = <1>;
 					#size-cells = <0>;
 					reg = <1>;
+
+					tcon0_out_dsi: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&dsi_in_tcon0>;
+						allwinner,tcon-channel = <1>;
+					};
 				};
 			};
 		};
@@ -829,6 +835,45 @@
 			status = "disabled";
 		};
 
+		dsi: dsi@1ca0000 {
+			compatible = "allwinner,sun50i-a64-mipi-dsi";
+			reg = <0x01ca0000 0x1000>;
+			interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_MIPI_DSI>;
+			clock-names = "bus";
+			resets = <&ccu RST_BUS_MIPI_DSI>;
+			phys = <&dphy>;
+			phy-names = "dphy";
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0>;
+
+					dsi_in_tcon0: endpoint {
+						remote-endpoint = <&tcon0_out_dsi>;
+					};
+				};
+			};
+		};
+
+		dphy: d-phy@1ca1000 {
+			compatible = "allwinner,sun50i-a64-mipi-dphy",
+				     "allwinner,sun6i-a31-mipi-dphy";
+			reg = <0x01ca1000 0x1000>;
+			clocks = <&ccu CLK_BUS_MIPI_DSI>,
+				 <&ccu CLK_DSI_DPHY>;
+			clock-names = "bus", "mod";
+			resets = <&ccu RST_BUS_MIPI_DSI>;
+			status = "disabled";
+			#phy-cells = <0>;
+		};
+
 		hdmi: hdmi@1ee0000 {
 			compatible = "allwinner,sun50i-a64-dw-hdmi",
 				     "allwinner,sun8i-a83t-dw-hdmi";
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 24/25] [DO NOT MERGE] arm64: dts: allwinner: bananapi-m64: Bananapi S070WV20-CT16 DSI panel

[Jagan Teki]

This patch add support for Bananapi S070WV20-CT16 DSI panel to
BPI-M64 board.

DSI panel connected via board DSI port with,
- DC1SW as AVDD supply
- DCDC1 as DVDD supply
- PD6 gpio for reset pin
- PD5 gpio for backlight enable pin
- PD7 gpio for backlight vdd supply

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v2:
- remove unit address 0
- add nodes in alphabetical order
- Use backlight instead of backlight_dsi
- add tested credit
Changes for v2:
- Use quadratic rule on pwm brightness

 .../dts/allwinner/sun50i-a64-bananapi-m64.dts | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index ef1c90401bb2..6cb010e3bbd9 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -45,6 +45,7 @@
 #include "sun50i-a64.dtsi"
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "BananaPi-M64";
@@ -56,6 +57,15 @@
 		serial1 = &uart1;
 	};
 
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&r_pwm 0 50000 PWM_POLARITY_INVERTED>;
+		brightness-levels = <1 2 4 8 16 32 64 128 512>;
+		default-brightness-level = <2>;
+		enable-gpios = <&pio 3 5 GPIO_ACTIVE_HIGH>; /* LCD-BL-EN: PD5 */
+		power-supply = <&reg_vdd_backlight>;
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
@@ -91,6 +101,15 @@
 		};
 	};
 
+	reg_vdd_backlight: vdd-backlight {
+		compatible = "regulator-fixed";
+		regulator-name = "vdd-backlight";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&pio 3 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PD7 */
+		enable-active-high;
+	};
+
 	wifi_pwrseq: wifi_pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */
@@ -101,6 +120,23 @@
 	status = "okay";
 };
 
+&dphy {
+	status = "okay";
+};
+
+&dsi {
+	status = "okay";
+
+	panel@0 {
+		compatible = "bananapi,s070wv20-ct16-icn6211";
+		reg = <0>;
+		avdd-supply = <&reg_dc1sw>;
+		dvdd-supply = <&reg_dldo1>;
+		reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD6 */
+		backlight = <&backlight>;
+	};
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -193,6 +229,12 @@
 	status = "okay";
 };
 
+&r_pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&r_pwm_pin>;
+	status = "okay";
+};
+
 &r_rsb {
 	status = "okay";
 
-- 
2.18.0.321.gffc6fa0e3

[PATCH v3 25/25] arm64: dts: allwinner: a64-amarula-relic: Enable Techstar TS8550B MIPI-DSI panel

[Jagan Teki]

Amarula A64-Relic board by default bound with Techstar TS8550B
MIPI-DSI panel, add support for it.

DSI panel connected via board DSI port with,
- DC1SW as AVDD supply
- DCDC2 as DVDD supply
- DCDC1 as VCC-DSI supply
- PD24 gpio for reset pin
- PD23 gpio for backlight enable pin

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- new patch

 .../allwinner/sun50i-a64-amarula-relic.dts    | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
index 6cb2b7f0c817..ecc0d8094815 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
@@ -9,6 +9,7 @@
 #include "sun50i-a64.dtsi"
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "Amarula A64-Relic";
@@ -18,6 +19,14 @@
 		serial0 = &uart0;
 	};
 
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
+		brightness-levels = <1 2 4 8 16 32 64 128 512>;
+		default-brightness-level = <2>;
+		enable-gpios = <&pio 3 23 GPIO_ACTIVE_HIGH>; /* LCD-BL-EN: PD23 */
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
@@ -30,6 +39,28 @@
 	};
 };
 
+&de {
+	status = "okay";
+};
+
+&dphy {
+	status = "okay";
+};
+
+&dsi {
+	vcc-dsi-supply = <&reg_dldo1>;
+	status = "okay";
+
+	panel@0 {
+		compatible = "techstar,ts8550b";
+		reg = <0>;
+		avdd-supply = <&reg_dc1sw>;
+		dvdd-supply = <&reg_dldo2>;
+		reset-gpios = <&pio 3 24 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD24 */
+		backlight = <&backlight>;
+	};
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -72,6 +103,12 @@
 	status = "okay";
 };
 
+&pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm_pin>;
+	status = "okay";
+};
+
 &r_rsb {
 	status = "okay";
 
@@ -107,6 +144,15 @@
 	regulator-name = "vcc-pll-avcc";
 };
 
+&reg_dc1sw {
+	/*
+	 * This regulator also indirectly drives the PD pingroup GPIOs,
+	 * which also controls the power LED.
+	 */
+	regulator-always-on;
+	regulator-name = "vcc-phy";
+};
+
 &reg_dcdc1 {
 	regulator-always-on;
 	regulator-min-microvolt = <3300000>;
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v3 00/25] drm/sun4i: Allwinner A64 MIPI-DSI support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

On Fri, Oct 26, 2018 at 08:13:19PM +0530, Jagan Teki wrote:
> This series fixed the issues related to work DSI on 2-lane panel
> which is reported on previous version[1].
> 
> Few comments from previous version still in discussion, but I just
> send this version just to group all working changes together.
> anyway I will fix in this in next version if any.
> 
> PLL_MIPI min rate is still weird, I tried many possible dclock rate
> from panel driver to satisfy manual suggested min rate 500MHz but
> none working so eventually moved 300MHz. any inputs on this area
> are welcome.
> 
> All these changes are tested in 2-lane, 4-lane MIPI-DSI panels.
> 
> If anyone wants to test, use this repo [2] with WIP-A64-DSI branch.

A changelog would be nice here, and usually you shouldn't send a new
version while there is ongoing discussion on the previous version.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 00/25] drm/sun4i: Allwinner A64 MIPI-DSI support

[Jagan Teki]

On Mon, Oct 29, 2018 at 2:45 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Fri, Oct 26, 2018 at 08:13:19PM +0530, Jagan Teki wrote:
> > This series fixed the issues related to work DSI on 2-lane panel
> > which is reported on previous version[1].
> >
> > Few comments from previous version still in discussion, but I just
> > send this version just to group all working changes together.
> > anyway I will fix in this in next version if any.
> >
> > PLL_MIPI min rate is still weird, I tried many possible dclock rate
> > from panel driver to satisfy manual suggested min rate 500MHz but
> > none working so eventually moved 300MHz. any inputs on this area
> > are welcome.
> >
> > All these changes are tested in 2-lane, 4-lane MIPI-DSI panels.
> >
> > If anyone wants to test, use this repo [2] with WIP-A64-DSI branch.
>
> A changelog would be nice here, and usually you shouldn't send a new
> version while there is ongoing discussion on the previous version.

True, I have few patches that fixes 2-lane device, I just grouped all
of these working-set together for further communication. mentioned the
same thing above.