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

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

[Jagan Teki]

This series add MIPI-DSI support on Allwinner A64. The same A31 controller
is reused and tweaked for A64 since the register space for both SoC's look same.

The current clock rate (270MHz) with nkm (5,2,11) from PLL_MIPI is unable to work
with A64 DSI block. I've tested with few changes to verify desired nkm divider
values (1, 2, 5) but with existing nkm divider logic(ccu_nkm_find_best) I'm unable 
to figure out the desired clock rate, anyone suggestion please let me know here.

Bananapi S070WV20-CT16 DSI panel with BPI-M64 board works fine,
but will figure out and fix the clock logic in next versions.

Thanks,
Jagan.

Jagan Teki (12):
  clk: sunxi-ng: a64: Fix gate bit of DSI DPHY
  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: Enable missing DSI bus clock
  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
  dt-bindings: panel: Add Bananapi S070WV20-CT16 MIPI-DSI panel bindings
  drm/panel: Add Bananapi S070WV20-CT16 MIPI-DSI panel driver
  arm64: dts: allwinner: a64: Add DSI pipeline
  arm64: dts: allwinner: bananapi-m64: Bananapi S070WV20-CT16 DSI panel

 .../panel/bananapi,s070wv20-ct16-dsi.txt      |  21 ++
 .../bindings/display/sunxi/sun6i-dsi.txt      |   1 +
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts |  42 +++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  44 +++
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c         |   2 +-
 drivers/gpu/drm/panel/Kconfig                 |   9 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-bananapi-s070wv20.c   | 336 ++++++++++++++++++
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c        |  78 +++-
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h        |   5 +
 10 files changed, 522 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16-dsi.txt
 create mode 100644 drivers/gpu/drm/panel/panel-bananapi-s070wv20.c

-- 
2.18.0.321.gffc6fa0e3

[PATCH 01/12] 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>
---
 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 5f80eb018014..b639e79641df 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 02/12] 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>
---
 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 03/12] 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>
---
 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 04/12] drm/sun4i: sun6i_mipi_dsi: Enable missing DSI bus clock

[Jagan Teki]

DSI bus_clk is already available in sun6i_dsi but missed to
get the clk and process for enable/disable.

This patch add support for it.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 8e9c76febca2..156b371243c6 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -1004,6 +1004,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 		return PTR_ERR(dsi->reset);
 	}
 
+	dsi->bus_clk = devm_clk_get(dev, "bus");
+	if (IS_ERR(dsi->bus_clk)) {
+		dev_err(dev, "Couldn't get the DSI bus clock\n");
+		return PTR_ERR(dsi->bus_clk);
+	}
+
 	if (dsi->variant->has_mod_clk) {
 		dsi->mod_clk = devm_clk_get(dev, "mod");
 		if (IS_ERR(dsi->mod_clk)) {
@@ -1012,6 +1018,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 		}
 	}
 
+	clk_prepare_enable(dsi->bus_clk);
 	/*
 	 * In order to operate properly, that clock seems to be always
 	 * set to 297MHz.
@@ -1065,6 +1072,7 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
 	sun6i_dphy_remove(dsi);
 	if (dsi->variant->has_mod_clk)
 		clk_rate_exclusive_put(dsi->mod_clk);
+	clk_disable_unprepare(dsi->bus_clk);
 
 	return 0;
 }
-- 
2.18.0.321.gffc6fa0e3

[PATCH 05/12] 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.

Also added error check for unsupporting transfer types this make
debugging easy for new transfer types.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 156b371243c6..1c7e42015645 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -869,6 +869,7 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
 		     SUN6I_DSI_CMD_CTL_TX_FLAG);
 
 	switch (msg->type) {
+	case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
 	case MIPI_DSI_DCS_SHORT_WRITE:
 	case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
 		ret = sun6i_dsi_dcs_write_short(dsi, msg);
@@ -885,6 +886,8 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
 		}
 
 	default:
+		dev_err(dsi->dev, "unsupported message type 0x%02x\n",
+			msg->type);
 		ret = -EINVAL;
 	}
 
-- 
2.18.0.321.gffc6fa0e3

[PATCH 06/12] drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation

[Jagan Teki]

According to horizontal and vertical timings are defined
per the diagram from include/drm/drm_modes.h

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

So, update SUN6I_DSI_BASIC_SIZE0_VBP calculation as
mode->vtotal - mode->vsync_end

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 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 1c7e42015645..599284971ab6 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 07/12] 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.

Since there no documentation for TCON_DRQ_REG(0x7c) register
this change is taken as reference from BPI-M64-bsp.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 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 599284971ab6..9918fdb990ff 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 08/12] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay

[Jagan Teki]

Accordingly to BPI-M64-bsp DE DSI code Video start delay
can be computed by subtracting total vertical timing with
front porch timing and with adding 1 delay line for TCON.

This patch simply add the start_delay logic from BPI-M64-bsp,
w/o this new computation, the DSI on A64 encounter vblank time out.

[CRTC:36:crtc-0] vblank wait timed out

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 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 9918fdb990ff..217db74c6dc3 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 09/12] dt-bindings: panel: Add Bananapi S070WV20-CT16 MIPI-DSI panel bindings

[Jagan Teki]

Bananapi S070WV20-CT16 is 800x480, 4-lane MIPI-DSI panel, the
same panel PCB comes with parallel RBG which is supported via
panel-simple with "bananapi,s070wv20-ct16" compatible.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 .../panel/bananapi,s070wv20-ct16-dsi.txt      | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16-dsi.txt

diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16-dsi.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16-dsi.txt
new file mode 100644
index 000000000000..c477163f7999
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16-dsi.txt
@@ -0,0 +1,21 @@
+Bananapi S070WV20 CT16 MIPI-DSI panel
+
+Required properties:
+- compatible: must be "bananapi,s070wv20-ct16-dsi"
+- 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.
+
+Example:
+panel@0 {
+	compatible = "bananapi,s070wv20-ct16-dsi";
+	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

[PATCH 10/12] drm/panel: Add Bananapi S070WV20-CT16 MIPI-DSI panel driver

[Jagan Teki]

Bananapi S070WV20-CT16 is 800x480, 4-lane MIPI-DSI panel which
can be used to connect via 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.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/gpu/drm/panel/Kconfig                 |   9 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-bananapi-s070wv20.c   | 336 ++++++++++++++++++
 3 files changed, 346 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-bananapi-s070wv20.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 6020c30a33b3..d25960a63b10 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
+	tristate "Bananapi S070WV20-CT16 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
+	  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..345b6475d3a4 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) += panel-bananapi-s070wv20.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.c b/drivers/gpu/drm/panel/panel-bananapi-s070wv20.c
new file mode 100644
index 000000000000..6c9adbec6dd8
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-bananapi-s070wv20.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 = 30000,
+	.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-dsi", },
+	{ }
+};
+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-dsi",
+		.of_match_table = s070wv20_of_match,
+	},
+};
+module_mipi_dsi_driver(s070wv20_driver);
+
+MODULE_AUTHOR("Jagan Teki <jagan@amarulasolutions.com>");
+MODULE_DESCRIPTION("Bananapi S070WV20-CT16 MIPI-DSI");
+MODULE_LICENSE("GPL v2");
-- 
2.18.0.321.gffc6fa0e3

[PATCH 11/12] 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>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index f3a66f888205..308eedc53150 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -320,6 +320,11 @@
 					#address-cells = <1>;
 					#size-cells = <0>;
 					reg = <1>;
+
+					tcon0_out_dsi: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&dsi_in_tcon0>;
+					};
 				};
 			};
 		};
@@ -818,6 +823,45 @@
 			#interrupt-cells = <3>;
 		};
 
+
+		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,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>;
+		};
+
 		pwm: pwm@1c21400 {
 			compatible = "allwinner,sun50i-a64-pwm",
 				     "allwinner,sun5i-a13-pwm";
-- 
2.18.0.321.gffc6fa0e3

[PATCH 12/12] 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>
---
 .../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..c0be949be68d 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,24 @@
 		serial1 = &uart1;
 	};
 
+	vdd_bl: regulator@0 {
+		compatible = "regulator-fixed";
+		regulator-name = "bl-3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&pio 3 7 GPIO_ACTIVE_HIGH>; /* PD7 */
+		enable-active-high;
+	};
+
+	backlight_dsi: backlight-dsi {
+		compatible = "pwm-backlight";
+		pwms = <&r_pwm 0 50000 PWM_POLARITY_INVERTED>;
+		brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
+		default-brightness-level = <2>;
+		enable-gpios = <&pio 3 5 GPIO_ACTIVE_HIGH>; /* PD5 */
+		power-supply = <&vdd_bl>;
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
@@ -101,6 +120,23 @@
 	status = "okay";
 };
 
+&dphy {
+	status = "okay";
+};
+
+&dsi {
+	status = "okay";
+
+	panel@0 {
+		compatible = "bananapi,s070wv20-ct16-dsi";
+		reg = <0>;
+		avdd-supply = <&reg_dc1sw>;
+		dvdd-supply = <&reg_dldo1>;
+		reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+		backlight = <&backlight_dsi>;
+	};
+};
+
 &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

Re: [PATCH 04/12] drm/sun4i: sun6i_mipi_dsi: Enable missing DSI bus clock

[Chen-Yu Tsai]

Hi,

On Thu, Sep 27, 2018 at 7:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> DSI bus_clk is already available in sun6i_dsi but missed to
> get the clk and process for enable/disable.
>
> This patch add support for it.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 8e9c76febca2..156b371243c6 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -1004,6 +1004,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>                 return PTR_ERR(dsi->reset);
>         }
>
> +       dsi->bus_clk = devm_clk_get(dev, "bus");
> +       if (IS_ERR(dsi->bus_clk)) {
> +               dev_err(dev, "Couldn't get the DSI bus clock\n");
> +               return PTR_ERR(dsi->bus_clk);
> +       }
> +

The DSI driver uses devm_regmap_init_mmio_clk, which enables the
clock behind the scenes when regmap access needs it enabled.

Did you have any issues without this patch?

ChenYu

>         if (dsi->variant->has_mod_clk) {
>                 dsi->mod_clk = devm_clk_get(dev, "mod");
>                 if (IS_ERR(dsi->mod_clk)) {
> @@ -1012,6 +1018,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>                 }
>         }
>
> +       clk_prepare_enable(dsi->bus_clk);
>         /*
>          * In order to operate properly, that clock seems to be always
>          * set to 297MHz.
> @@ -1065,6 +1072,7 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
>         sun6i_dphy_remove(dsi);
>         if (dsi->variant->has_mod_clk)
>                 clk_rate_exclusive_put(dsi->mod_clk);
> +       clk_disable_unprepare(dsi->bus_clk);
>
>         return 0;
>  }
> --
> 2.18.0.321.gffc6fa0e3
>

Re: [PATCH 04/12] drm/sun4i: sun6i_mipi_dsi: Enable missing DSI bus clock

[Jagan Teki]

On Thu, Sep 27, 2018 at 6:13 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> Hi,
>
> On Thu, Sep 27, 2018 at 7:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > DSI bus_clk is already available in sun6i_dsi but missed to
> > get the clk and process for enable/disable.
> >
> > This patch add support for it.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 8e9c76febca2..156b371243c6 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -1004,6 +1004,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
> >                 return PTR_ERR(dsi->reset);
> >         }
> >
> > +       dsi->bus_clk = devm_clk_get(dev, "bus");
> > +       if (IS_ERR(dsi->bus_clk)) {
> > +               dev_err(dev, "Couldn't get the DSI bus clock\n");
> > +               return PTR_ERR(dsi->bus_clk);
> > +       }
> > +
>
> The DSI driver uses devm_regmap_init_mmio_clk, which enables the
> clock behind the scenes when regmap access needs it enabled.
>
> Did you have any issues without this patch?

I'm unable to read register values via devmem, I see all 0's for all
dsi reg space.

Re: [linux-sunxi] Re: [PATCH 04/12] drm/sun4i: sun6i_mipi_dsi: Enable missing DSI bus clock

[Chen-Yu Tsai]

On Thu, Sep 27, 2018 at 9:44 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Thu, Sep 27, 2018 at 6:13 PM Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > Hi,
> >
> > On Thu, Sep 27, 2018 at 7:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > DSI bus_clk is already available in sun6i_dsi but missed to
> > > get the clk and process for enable/disable.
> > >
> > > This patch add support for it.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > index 8e9c76febca2..156b371243c6 100644
> > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > @@ -1004,6 +1004,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
> > >                 return PTR_ERR(dsi->reset);
> > >         }
> > >
> > > +       dsi->bus_clk = devm_clk_get(dev, "bus");
> > > +       if (IS_ERR(dsi->bus_clk)) {
> > > +               dev_err(dev, "Couldn't get the DSI bus clock\n");
> > > +               return PTR_ERR(dsi->bus_clk);
> > > +       }
> > > +
> >
> > The DSI driver uses devm_regmap_init_mmio_clk, which enables the
> > clock behind the scenes when regmap access needs it enabled.
> >
> > Did you have any issues without this patch?
>
> I'm unable to read register values via devmem, I see all 0's for all
> dsi reg space.

Well that is to be expected given how mmio with clk regmap works.
Did you encounter any other problems besides this?

FYI you can dump regmap contents using /sys/kernel/debug/regmap/*/registers

ChenYu

Re: [PATCH 06/12] drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation

[Maxime Ripard]

On Thu, Sep 27, 2018 at 05:18:44PM +0530, Jagan Teki wrote:
> According to horizontal and vertical timings are defined
> per the diagram from include/drm/drm_modes.h
> 
> Back porch = [hv]total - [hv]sync_end
>
> So, update SUN6I_DSI_BASIC_SIZE0_VBP calculation as
> mode->vtotal - mode->vsync_end
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  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 1c7e42015645..599284971ab6 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));

Is it purely theoretical, or did you find some source that back that?

Maxime

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

Re: [PATCH 06/12] drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation

[Jagan Teki]

On Thu, Sep 27, 2018 at 8:51 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Thu, Sep 27, 2018 at 05:18:44PM +0530, Jagan Teki wrote:
> > According to horizontal and vertical timings are defined
> > per the diagram from include/drm/drm_modes.h
> >
> > Back porch = [hv]total - [hv]sync_end
> >
> > So, update SUN6I_DSI_BASIC_SIZE0_VBP calculation as
> > mode->vtotal - mode->vsync_end
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  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 1c7e42015645..599284971ab6 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));
>
> Is it purely theoretical, or did you find some source that back that?

VSA is done as per that, sync_end - sync start would give sync time.
VBP also done in BPI-M64-bsp[1] which results back porch existing code
results fron porch.

[1] https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L955

Re: [linux-sunxi] Re: [PATCH 04/12] drm/sun4i: sun6i_mipi_dsi: Enable missing DSI bus clock

[Jagan Teki]

On Thu, Sep 27, 2018 at 7:47 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Thu, Sep 27, 2018 at 9:44 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > On Thu, Sep 27, 2018 at 6:13 PM Chen-Yu Tsai <wens@csie.org> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, Sep 27, 2018 at 7:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > >
> > > > DSI bus_clk is already available in sun6i_dsi but missed to
> > > > get the clk and process for enable/disable.
> > > >
> > > > This patch add support for it.
> > > >
> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > ---
> > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > index 8e9c76febca2..156b371243c6 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > @@ -1004,6 +1004,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
> > > >                 return PTR_ERR(dsi->reset);
> > > >         }
> > > >
> > > > +       dsi->bus_clk = devm_clk_get(dev, "bus");
> > > > +       if (IS_ERR(dsi->bus_clk)) {
> > > > +               dev_err(dev, "Couldn't get the DSI bus clock\n");
> > > > +               return PTR_ERR(dsi->bus_clk);
> > > > +       }
> > > > +
> > >
> > > The DSI driver uses devm_regmap_init_mmio_clk, which enables the
> > > clock behind the scenes when regmap access needs it enabled.
> > >
> > > Did you have any issues without this patch?
> >
> > I'm unable to read register values via devmem, I see all 0's for all
> > dsi reg space.
>
> Well that is to be expected given how mmio with clk regmap works.
> Did you encounter any other problems besides this?

no

>
> FYI you can dump regmap contents using /sys/kernel/debug/regmap/*/registers

Thanks, but ccu not using regmap I believe so can't see bus enabled
bit set via devmap
# devmem 0x01C20060
0x33800000

Re: [linux-sunxi] Re: [PATCH 04/12] drm/sun4i: sun6i_mipi_dsi: Enable missing DSI bus clock

[Chen-Yu Tsai]

On Fri, Sep 28, 2018 at 12:26 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Thu, Sep 27, 2018 at 7:47 PM Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > On Thu, Sep 27, 2018 at 9:44 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > On Thu, Sep 27, 2018 at 6:13 PM Chen-Yu Tsai <wens@csie.org> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Thu, Sep 27, 2018 at 7:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > > >
> > > > > DSI bus_clk is already available in sun6i_dsi but missed to
> > > > > get the clk and process for enable/disable.
> > > > >
> > > > > This patch add support for it.
> > > > >
> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > ---
> > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 8 ++++++++
> > > > >  1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > > index 8e9c76febca2..156b371243c6 100644
> > > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > > @@ -1004,6 +1004,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
> > > > >                 return PTR_ERR(dsi->reset);
> > > > >         }
> > > > >
> > > > > +       dsi->bus_clk = devm_clk_get(dev, "bus");
> > > > > +       if (IS_ERR(dsi->bus_clk)) {
> > > > > +               dev_err(dev, "Couldn't get the DSI bus clock\n");
> > > > > +               return PTR_ERR(dsi->bus_clk);
> > > > > +       }
> > > > > +
> > > >
> > > > The DSI driver uses devm_regmap_init_mmio_clk, which enables the
> > > > clock behind the scenes when regmap access needs it enabled.
> > > >
> > > > Did you have any issues without this patch?
> > >
> > > I'm unable to read register values via devmem, I see all 0's for all
> > > dsi reg space.
> >
> > Well that is to be expected given how mmio with clk regmap works.
> > Did you encounter any other problems besides this?
>
> no
>
> >
> > FYI you can dump regmap contents using /sys/kernel/debug/regmap/*/registers
>
> Thanks, but ccu not using regmap I believe so can't see bus enabled
> bit set via devmap
> # devmem 0x01C20060
> 0x33800000

That is perfectly normal. It is only enabled when there are accesses to the
DSI regmap.

ChenYu

Re: [PATCH 07/12] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits

[Maxime Ripard]

On Thu, Sep 27, 2018 at 05:18:45PM +0530, Jagan Teki wrote:
> TCON DRQ set bits for non-burst DSI mode can computed via
> horizontal front porch instead of front porch + sync timings.
>
> Since there no documentation for TCON_DRQ_REG(0x7c) register
> this change is taken as reference from BPI-M64-bsp.

Detailing more what the issue is would be great.

> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  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 599284971ab6..9918fdb990ff 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;

The computation here is in the A64 driver:

if ((panel->lcd_ht - panel->lcd_x - panel->lcd_hbp) < 21) {
	dsi_dev[sel]->dsi_tcon_drq.bits.drq_mode = 0;
} else {
	dsi_dev[sel]->dsi_tcon_drq.bits.drq_set	=
		(panel->lcd_ht-panel->lcd_x-panel->lcd_hbp-20) *
		dsi_pixel_bits[panel->lcd_dsi_format]/(8*4);
}

It is testing that the sync + front porch is smaller than 21, and
otherwise sets the drq.

> -	if ((mode->hsync_end - mode->hdisplay) > 20) {

My code here is testing that the difference between hsync_end and
hdisplay is superior to 20, and sets the DRQ if true. The condition is
reversed, but otherwise, that difference is the front porch plus the
sync length.

> +	if ((mode->hsync_start - mode->hdisplay) > 20) {

However, you are testing for just the front porch, unlike what your
commit log is saying, and unlike what allwinner's code is saying. So
this deserves some explanation.

Maxime

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

Re: [PATCH 08/12] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay

[Maxime Ripard]

On Thu, Sep 27, 2018 at 05:18:46PM +0530, Jagan Teki wrote:
> Accordingly to BPI-M64-bsp DE DSI code Video start delay
> can be computed by subtracting total vertical timing with
> front porch timing and with adding 1 delay line for TCON.

This is what the current code is doing as well.

> This patch simply add the start_delay logic from BPI-M64-bsp,
> w/o this new computation, the DSI on A64 encounter vblank time out.
> 
> [CRTC:36:crtc-0] vblank wait timed out
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  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 9918fdb990ff..217db74c6dc3 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;

Again, this is wrong and contrary to what your commit log is saying.

The Allwinner code has:

u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp;
u32 dsi_start_delay = panel->lcd_vt - vfp + 1;

So, essentially:

vtotal - (vtotal - vdisplay - back porch) + 1

The backporch is hsync_total - hsync_end, so we end up, removing the
addition / removal of vtotal, with:

vdisplay - (vsync_total - vsync_end) + 1

The formula used there looks indeed different, but unlike what you
were saying.

> +	u32 start_delay;
> +
> +	start_delay = mode->vtotal - vfp + 1;
> +	if (start_delay > mode->vtotal)
> +		start_delay -= mode->vtotal;
> +
> +	if (!start_delay)
> +		start_delay = 1;
> +

I guess that it's actually the clamping that fixes thing. It should be
mentionned in your commit log.

Maxime

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

Re: [PATCH 11/12] arm64: dts: allwinner: a64: Add DSI pipeline

[Maxime Ripard]

On Thu, Sep 27, 2018 at 05:18:49PM +0530, Jagan Teki wrote:
> 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>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index f3a66f888205..308eedc53150 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -320,6 +320,11 @@
>  					#address-cells = <1>;
>  					#size-cells = <0>;
>  					reg = <1>;
> +
> +					tcon0_out_dsi: endpoint@1 {
> +						reg = <1>;
> +						remote-endpoint = <&dsi_in_tcon0>;
> +					};
>  				};
>  			};
>  		};
> @@ -818,6 +823,45 @@
>  			#interrupt-cells = <3>;
>  		};
>  
> +
> +		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,sun6i-a31-mipi-dphy";

You should add an A64 compatible there.

Thanks!
Maxime

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

Re: [PATCH 12/12] arm64: dts: allwinner: bananapi-m64: Bananapi S070WV20-CT16 DSI panel

[Maxime Ripard]

On Thu, Sep 27, 2018 at 05:18:50PM +0530, Jagan Teki wrote:
> 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>
> ---
>  .../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..c0be949be68d 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,24 @@
>  		serial1 = &uart1;
>  	};
>  
> +	vdd_bl: regulator@0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "bl-3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&pio 3 7 GPIO_ACTIVE_HIGH>; /* PD7 */
> +		enable-active-high;
> +	};
> +
> +	backlight_dsi: backlight-dsi {
> +		compatible = "pwm-backlight";
> +		pwms = <&r_pwm 0 50000 PWM_POLARITY_INVERTED>;
> +		brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;

The brightness levels should be increasing linearly from a user point
of view. Usually, with a PWM backlight, a quadratic rule works much
better.

MMaxime

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

Re: [PATCH 05/12] drm/sun4i: sun6i_mipi_dsi: Add DSI Generic short write 2 param transfer

[Maxime Ripard]

On Thu, Sep 27, 2018 at 05:18:43PM +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.
> 
> Also added error check for unsupporting transfer types this make
> debugging easy for new transfer types.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 156b371243c6..1c7e42015645 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -869,6 +869,7 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
>  		     SUN6I_DSI_CMD_CTL_TX_FLAG);
>  
>  	switch (msg->type) {
> +	case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
>  	case MIPI_DSI_DCS_SHORT_WRITE:
>  	case MIPI_DSI_DCS_SHORT_WRITE_PARAM:

You should order them by alphabetical order.

>  		ret = sun6i_dsi_dcs_write_short(dsi, msg);
> @@ -885,6 +886,8 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
>  		}
>  
>  	default:
> +		dev_err(dsi->dev, "unsupported message type 0x%02x\n",
> +			msg->type);

And this isn't an error check.

Maxime

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

Re: [PATCH 08/12] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay

[Jagan Teki]

On Thu, Sep 27, 2018 at 10:44 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> On Thu, Sep 27, 2018 at 05:18:46PM +0530, Jagan Teki wrote:
> > Accordingly to BPI-M64-bsp DE DSI code Video start delay
> > can be computed by subtracting total vertical timing with
> > front porch timing and with adding 1 delay line for TCON.
>
> This is what the current code is doing as well.

The current code
return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1;

(mode->vsync_end - mode->vdisplay) = front porch + sync

but I'm updating here only front porch.

>
> > This patch simply add the start_delay logic from BPI-M64-bsp,
> > w/o this new computation, the DSI on A64 encounter vblank time out.
> >
> > [CRTC:36:crtc-0] vblank wait timed out
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  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 9918fdb990ff..217db74c6dc3 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;

let me explain this.

Actual code from Allwinner
u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp;

So,

=> (panel->lcd_vt - panel->lcd_y) - (panel->lcd_vbp)
=> (front porch + sync + back porch) - (back porch + sync)
=> front porch + sync + back porch - back porch - sync
=> front porch
=> mode->vsync_start - mode->vdisplay

Hope it clear.

Re: [PATCH 05/12] drm/sun4i: sun6i_mipi_dsi: Add DSI Generic short write 2 param transfer

[Jagan Teki]

On Thu, Sep 27, 2018 at 10:48 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> On Thu, Sep 27, 2018 at 05:18:43PM +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.
> >
> > Also added error check for unsupporting transfer types this make
> > debugging easy for new transfer types.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 156b371243c6..1c7e42015645 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -869,6 +869,7 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
> >                    SUN6I_DSI_CMD_CTL_TX_FLAG);
> >
> >       switch (msg->type) {
> > +     case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
> >       case MIPI_DSI_DCS_SHORT_WRITE:
> >       case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
>
> You should order them by alphabetical order.

I just placed it with transfer code, ok will add it alphabetical

>
> >               ret = sun6i_dsi_dcs_write_short(dsi, msg);
> > @@ -885,6 +886,8 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
> >               }
> >
> >       default:
> > +             dev_err(dsi->dev, "unsupported message type 0x%02x\n",
> > +                     msg->type);
>
> And this isn't an error check.

But unsupported message type by sun6i_dsi should be an error
eventually isn't it? and we can easily figure out where the error
trigger.

Re: [PATCH 07/12] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits

[Jagan Teki]

On Thu, Sep 27, 2018 at 10:28 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> On Thu, Sep 27, 2018 at 05:18:45PM +0530, Jagan Teki wrote:
> > TCON DRQ set bits for non-burst DSI mode can computed via
> > horizontal front porch instead of front porch + sync timings.
> >
> > Since there no documentation for TCON_DRQ_REG(0x7c) register
> > this change is taken as reference from BPI-M64-bsp.
>
> Detailing more what the issue is would be great.
>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  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 599284971ab6..9918fdb990ff 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;
>
> The computation here is in the A64 driver:
>
> if ((panel->lcd_ht - panel->lcd_x - panel->lcd_hbp) < 21) {
>         dsi_dev[sel]->dsi_tcon_drq.bits.drq_mode = 0;
> } else {
>         dsi_dev[sel]->dsi_tcon_drq.bits.drq_set =
>                 (panel->lcd_ht-panel->lcd_x-panel->lcd_hbp-20) *
>                 dsi_pixel_bits[panel->lcd_dsi_format]/(8*4);
> }
>
> It is testing that the sync + front porch is smaller than 21, and
> otherwise sets the drq.
>
> > -     if ((mode->hsync_end - mode->hdisplay) > 20) {
>
> My code here is testing that the difference between hsync_end and
> hdisplay is superior to 20, and sets the DRQ if true. The condition is
> reversed, but otherwise, that difference is the front porch plus the
> sync length.

True, I understand this, but does drq setting here is specific to SoC?
I thought of finding DRQ in A31 BSP but I couldn't find the code. do
you have bsp somewhere in github?

>
> > +     if ((mode->hsync_start - mode->hdisplay) > 20) {
>
> However, you are testing for just the front porch, unlike what your
> commit log is saying, and unlike what allwinner's code is saying. So
> this deserves some explanation.

but A64 is doing this, do you think it's completely A64 specific or
testing panel with front porch drq?

Re: [PATCH 10/12] drm/panel: Add Bananapi S070WV20-CT16 MIPI-DSI panel driver

[Chen-Yu Tsai]

On Thu, Sep 27, 2018 at 7:50 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Bananapi S070WV20-CT16 is 800x480, 4-lane MIPI-DSI panel which
> can be used to connect via 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.

As I mentioned before, this PCB uses a ICN6211 MIPI-DSI to RGB bridge.
Please model it as such.

> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                 |   9 +
>  drivers/gpu/drm/panel/Makefile                |   1 +
>  .../gpu/drm/panel/panel-bananapi-s070wv20.c   | 336 ++++++++++++++++++
>  3 files changed, 346 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-bananapi-s070wv20.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 6020c30a33b3..d25960a63b10 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
> +       tristate "Bananapi S070WV20-CT16 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
> +         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..345b6475d3a4 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) += panel-bananapi-s070wv20.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.c b/drivers/gpu/drm/panel/panel-bananapi-s070wv20.c
> new file mode 100644
> index 000000000000..6c9adbec6dd8
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-bananapi-s070wv20.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__} }
> +

Using the timings from below, and comparing with

   https://shiminblog.github.io/mipi-to-rgb-lcd/

> +static const struct s070wv20_init_cmd s070wv20_init_cmds[] = {
> +       S070WV20_INIT_CMD(0x7A, 0xC1),

> +       S070WV20_INIT_CMD(0x20, 0x20),

This is likely the lower 8 bits of .hdisplay.

> +       S070WV20_INIT_CMD(0x21, 0xE0),

Lower 8 bits of .vdisplay

> +       S070WV20_INIT_CMD(0x22, 0x13),

Might be ((.hdisplay >> 8) & 0xf | (.vdisplay >> 8) & 0xf) << 4).

> +       S070WV20_INIT_CMD(0x23, 0x28),

HFP, or horizontal front porch. 40 in this case.

> +       S070WV20_INIT_CMD(0x24, 0x30),

HSYNC, or horizontal sync pulse length. 48 in this case.

> +       S070WV20_INIT_CMD(0x25, 0x28),

HBP, or horizontal back porch. 40 in this case.

> +       S070WV20_INIT_CMD(0x26, 0x00),

Unknown. Possibly RGB phase shift?

> +       S070WV20_INIT_CMD(0x27, 0x0D),

VFP, or vertical front porch. 13 in this case.

> +       S070WV20_INIT_CMD(0x28, 0x03),

This might be VSYNC, judging from the grouping. 3 in this case.

> +       S070WV20_INIT_CMD(0x29, 0x1D),

VBP, or vertical back porch. 29 in this case.

> +       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),

This might be related to number of MIPI lanes. I saw 0x88 for 2-lane devices.

> +       S070WV20_INIT_CMD(0xB6, 0x20),
> +       S070WV20_INIT_CMD(0x51, 0x20),
> +       S070WV20_INIT_CMD(0x09, 0x10),

The last three seem standardized across all ICN6211 drivers I could
find on github.

Other drivers / init sequences I found:

https://github.com/yesnoandor/x300/blob/master/kernel/arch/arm/boot/dts/erobbing/x300/x300.dtsi#L81
https://github.com/wxzed/Raspberry_5MIPI_Display/blob/master/I2C_Slave/USER/main.c#L15

https://github.com/eliot-shao/qcom/blob/master/icn6211_cxn0102/kernel/drivers/video/msm/mdss/mdss_i2c_interface.c#L152
matches timings for
https://github.com/eliot-shao/qcom/blob/master/icn6211_cxn0102/kernel/arch/arm/boot/dts/qcom/dsi-mipi-2-rgb_1280p_video.dtsi#L20

https://github.com/zestroly/micromat/blob/master/test/raspberry/ICN6211.cpp#L169

I also reached out to Bananapi to see if they have more information on
this chip.

ChenYu

> +};
> +
> +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 = 30000,
> +       .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-dsi", },
> +       { }
> +};
> +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-dsi",
> +               .of_match_table = s070wv20_of_match,
> +       },
> +};
> +module_mipi_dsi_driver(s070wv20_driver);
> +
> +MODULE_AUTHOR("Jagan Teki <jagan@amarulasolutions.com>");
> +MODULE_DESCRIPTION("Bananapi S070WV20-CT16 MIPI-DSI");
> +MODULE_LICENSE("GPL v2");
> --
> 2.18.0.321.gffc6fa0e3
>

Re: [PATCH 05/12] drm/sun4i: sun6i_mipi_dsi: Add DSI Generic short write 2 param transfer

[Maxime Ripard]

On Thu, Sep 27, 2018 at 11:06:50PM +0530, Jagan Teki wrote:
> >
> > >               ret = sun6i_dsi_dcs_write_short(dsi, msg);
> > > @@ -885,6 +886,8 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
> > >               }
> > >
> > >       default:
> > > +             dev_err(dsi->dev, "unsupported message type 0x%02x\n",
> > > +                     msg->type);
> >
> > And this isn't an error check.
> 
> But unsupported message type by sun6i_dsi should be an error
> eventually isn't it?

It's already an error condition. What you're adding, and unlike what
your commit log says, is not an error check...

> and we can easily figure out where the error trigger.

... but instead an error message.

That's definitely not the same thing, and I'm not sure we actually
need it. If a driver requests multiple transfers that are unsupported,
we'll end up spaming the kernel logs, especially when it can and
should be checked in the driver doing those transfers.

Maxime

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

Re: [PATCH 06/12] drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation

[Maxime Ripard]

On Thu, Sep 27, 2018 at 09:50:34PM +0530, Jagan Teki wrote:
> On Thu, Sep 27, 2018 at 8:51 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Thu, Sep 27, 2018 at 05:18:44PM +0530, Jagan Teki wrote:
> > > According to horizontal and vertical timings are defined
> > > per the diagram from include/drm/drm_modes.h
> > >
> > > Back porch = [hv]total - [hv]sync_end
> > >
> > > So, update SUN6I_DSI_BASIC_SIZE0_VBP calculation as
> > > mode->vtotal - mode->vsync_end
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > >  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 1c7e42015645..599284971ab6 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));
> >
> > Is it purely theoretical, or did you find some source that back that?
> 
> VSA is done as per that, sync_end - sync start would give sync time.

That's a different register though.

> VBP also done in BPI-M64-bsp[1] which results back porch existing code
> results fron porch.
> 
> [1] https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L955

That code does back porch - sync length. Such a calculation doesn't
make much sense as is, but you're saying that it results in the front
porch. Again, what makes you say that?

Maxime

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

Re: [PATCH 08/12] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay

[Maxime Ripard]

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

On Thu, Sep 27, 2018 at 11:03:19PM +0530, Jagan Teki wrote:
> On Thu, Sep 27, 2018 at 10:44 PM Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> >
> > On Thu, Sep 27, 2018 at 05:18:46PM +0530, Jagan Teki wrote:
> > > Accordingly to BPI-M64-bsp DE DSI code Video start delay
> > > can be computed by subtracting total vertical timing with
> > > front porch timing and with adding 1 delay line for TCON.
> >
> > This is what the current code is doing as well.
> 
> The current code
> return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1;
> 
> (mode->vsync_end - mode->vdisplay) = front porch + sync
> 
> but I'm updating here only front porch.
> 
> >
> > > This patch simply add the start_delay logic from BPI-M64-bsp,
> > > w/o this new computation, the DSI on A64 encounter vblank time out.
> > >
> > > [CRTC:36:crtc-0] vblank wait timed out
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > >  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 9918fdb990ff..217db74c6dc3 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;
> 
> let me explain this.
> 
> Actual code from Allwinner
> u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp;
> 
> So,
> 
> => (panel->lcd_vt - panel->lcd_y) - (panel->lcd_vbp)
> => (front porch + sync + back porch) - (back porch + sync)

Unless Allwinner is doing something fishy, in which case that should
be mentionned, the back porch doesn't contain the sync pulse.

Maxime

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

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

Re: [linux-sunxi] Re: [PATCH 08/12] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay

[Jagan Teki]

On Saturday 29 September 2018 08:57 PM, Maxime Ripard wrote:
> On Thu, Sep 27, 2018 at 11:03:19PM +0530, Jagan Teki wrote:
>> On Thu, Sep 27, 2018 at 10:44 PM Maxime Ripard
>> <maxime.ripard@bootlin.com> wrote:
>>>
>>> On Thu, Sep 27, 2018 at 05:18:46PM +0530, Jagan Teki wrote:
>>>> Accordingly to BPI-M64-bsp DE DSI code Video start delay
>>>> can be computed by subtracting total vertical timing with
>>>> front porch timing and with adding 1 delay line for TCON.
>>>
>>> This is what the current code is doing as well.
>>
>> The current code
>> return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1;
>>
>> (mode->vsync_end - mode->vdisplay) = front porch + sync
>>
>> but I'm updating here only front porch.
>>
>>>
>>>> This patch simply add the start_delay logic from BPI-M64-bsp,
>>>> w/o this new computation, the DSI on A64 encounter vblank time out.
>>>>
>>>> [CRTC:36:crtc-0] vblank wait timed out
>>>>
>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>> ---
>>>>   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 9918fdb990ff..217db74c6dc3 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;
>>
>> let me explain this.
>>
>> Actual code from Allwinner
>> u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp;
>>
>> So,
>>
>> => (panel->lcd_vt - panel->lcd_y) - (panel->lcd_vbp)
>> => (front porch + sync + back porch) - (back porch + sync)
> 
> Unless Allwinner is doing something fishy, in which case that should
> be mentionned, the back porch doesn't contain the sync pulse.

As per as I understand panel->lcd_vbp is not back porch timings value 
which i used by drm. It is BSP DTS property value and actual back porch 
is calculated as "panel->lcd_vbp - panel->sync"

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

Re: [linux-sunxi] Re: [PATCH 06/12] drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation

[Jagan Teki]

On Saturday 29 September 2018 07:23 PM, Maxime Ripard wrote:
> On Thu, Sep 27, 2018 at 09:50:34PM +0530, Jagan Teki wrote:
>> On Thu, Sep 27, 2018 at 8:51 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>>>
>>> On Thu, Sep 27, 2018 at 05:18:44PM +0530, Jagan Teki wrote:
>>>> According to horizontal and vertical timings are defined
>>>> per the diagram from include/drm/drm_modes.h
>>>>
>>>> Back porch = [hv]total - [hv]sync_end
>>>>
>>>> So, update SUN6I_DSI_BASIC_SIZE0_VBP calculation as
>>>> mode->vtotal - mode->vsync_end
>>>>
>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>> ---
>>>>   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 1c7e42015645..599284971ab6 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));
>>>
>>> Is it purely theoretical, or did you find some source that back that?
>>
>> VSA is done as per that, sync_end - sync start would give sync time.
> 
> That's a different register though.
> 
>> VBP also done in BPI-M64-bsp[1] which results back porch existing code
>> results fron porch.
>>
>> [1] https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L955
> 
> That code does back porch - sync length. Such a calculation doesn't
> make much sense as is, but you're saying that it results in the front
> porch. Again, what makes you say that?

No what code shows is not a real back porch value used for drm it's 
panel back porch value which is a DTS property in BSP.
(I made wrong comment on previous mail as front porch, sorry)

here is the real code

from 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;


u32 vbp = panel->lcd_vbp;
u32 vspw = panel->lcd_vspw;
dsi_dev[sel]->dsi_basic_size0.bits.vbp = vbp-vspw;

So,

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

VSA, which a proper value.

dsi_dev[sel]->dsi_basic_size0.bits.vsa = vspw;
=> panel_info->lcd_vspw;
=> timmings->ver_sync_time
=> mode->vsync_end - mode->vsync_start

Re: [PATCH 07/12] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits

[Maxime Ripard]

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

On Thu, Sep 27, 2018 at 11:15:50PM +0530, Jagan Teki wrote:
> On Thu, Sep 27, 2018 at 10:28 PM Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> >
> > On Thu, Sep 27, 2018 at 05:18:45PM +0530, Jagan Teki wrote:
> > > TCON DRQ set bits for non-burst DSI mode can computed via
> > > horizontal front porch instead of front porch + sync timings.
> > >
> > > Since there no documentation for TCON_DRQ_REG(0x7c) register
> > > this change is taken as reference from BPI-M64-bsp.
> >
> > Detailing more what the issue is would be great.
> >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > >  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 599284971ab6..9918fdb990ff 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;
> >
> > The computation here is in the A64 driver:
> >
> > if ((panel->lcd_ht - panel->lcd_x - panel->lcd_hbp) < 21) {
> >         dsi_dev[sel]->dsi_tcon_drq.bits.drq_mode = 0;
> > } else {
> >         dsi_dev[sel]->dsi_tcon_drq.bits.drq_set =
> >                 (panel->lcd_ht-panel->lcd_x-panel->lcd_hbp-20) *
> >                 dsi_pixel_bits[panel->lcd_dsi_format]/(8*4);
> > }
> >
> > It is testing that the sync + front porch is smaller than 21, and
> > otherwise sets the drq.
> >
> > > -     if ((mode->hsync_end - mode->hdisplay) > 20) {
> >
> > My code here is testing that the difference between hsync_end and
> > hdisplay is superior to 20, and sets the DRQ if true. The condition is
> > reversed, but otherwise, that difference is the front porch plus the
> > sync length.
> 
> True, I understand this, but does drq setting here is specific to SoC?
> I thought of finding DRQ in A31 BSP but I couldn't find the code. do
> you have bsp somewhere in github?
> 
> >
> > > +     if ((mode->hsync_start - mode->hdisplay) > 20) {
> >
> > However, you are testing for just the front porch, unlike what your
> > commit log is saying, and unlike what allwinner's code is saying. So
> > this deserves some explanation.
> 
> but A64 is doing this, do you think it's completely A64 specific or
> testing panel with front porch drq?

See the above code excerpt:
panel->lcd_ht - panel->lcd_x - panel->lcd_hbp

This is hsync + front porch. Not the sole front porch. So no, it's not
doing this.

Maxime


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

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

Re: [linux-sunxi] Re: [PATCH 08/12] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay

[Maxime Ripard]

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

On Mon, Oct 01, 2018 at 01:25:59PM +0530, Jagan Teki wrote:
> On Saturday 29 September 2018 08:57 PM, Maxime Ripard wrote:
> > On Thu, Sep 27, 2018 at 11:03:19PM +0530, Jagan Teki wrote:
> > > On Thu, Sep 27, 2018 at 10:44 PM Maxime Ripard
> > > <maxime.ripard@bootlin.com> wrote:
> > > > 
> > > > On Thu, Sep 27, 2018 at 05:18:46PM +0530, Jagan Teki wrote:
> > > > > Accordingly to BPI-M64-bsp DE DSI code Video start delay
> > > > > can be computed by subtracting total vertical timing with
> > > > > front porch timing and with adding 1 delay line for TCON.
> > > > 
> > > > This is what the current code is doing as well.
> > > 
> > > The current code
> > > return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1;
> > > 
> > > (mode->vsync_end - mode->vdisplay) = front porch + sync
> > > 
> > > but I'm updating here only front porch.
> > > 
> > > > 
> > > > > This patch simply add the start_delay logic from BPI-M64-bsp,
> > > > > w/o this new computation, the DSI on A64 encounter vblank time out.
> > > > > 
> > > > > [CRTC:36:crtc-0] vblank wait timed out
> > > > > 
> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > ---
> > > > >   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 9918fdb990ff..217db74c6dc3 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;
> > > 
> > > let me explain this.
> > > 
> > > Actual code from Allwinner
> > > u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp;
> > > 
> > > So,
> > > 
> > > => (panel->lcd_vt - panel->lcd_y) - (panel->lcd_vbp)
> > > => (front porch + sync + back porch) - (back porch + sync)
> > 
> > Unless Allwinner is doing something fishy, in which case that should
> > be mentionned, the back porch doesn't contain the sync pulse.
> 
> As per as I understand panel->lcd_vbp is not back porch timings value which
> i used by drm. It is BSP DTS property value and actual back porch is
> calculated as "panel->lcd_vbp - panel->sync"
> 
> timmings->ver_sync_time= panel_info->lcd_vspw;
> timmings->ver_back_porch= panel_info->lcd_vbp-panel_info->lcd_vspw;

Then this is what you should have started with in your commit
log. Where is that code coming from? Have you been able to confirm
that with an oscilloscope?

Maxime

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

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

Re: [linux-sunxi] Re: [PATCH 07/12] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits

[Jagan Teki]

On Tuesday 02 October 2018 06:50 PM, Maxime Ripard wrote:
> On Thu, Sep 27, 2018 at 11:15:50PM +0530, Jagan Teki wrote:
>> On Thu, Sep 27, 2018 at 10:28 PM Maxime Ripard
>> <maxime.ripard@bootlin.com> wrote:
>>>
>>> On Thu, Sep 27, 2018 at 05:18:45PM +0530, Jagan Teki wrote:
>>>> TCON DRQ set bits for non-burst DSI mode can computed via
>>>> horizontal front porch instead of front porch + sync timings.
>>>>
>>>> Since there no documentation for TCON_DRQ_REG(0x7c) register
>>>> this change is taken as reference from BPI-M64-bsp.
>>>
>>> Detailing more what the issue is would be great.
>>>
>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>> ---
>>>>   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 599284971ab6..9918fdb990ff 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;
>>>
>>> The computation here is in the A64 driver:
>>>
>>> if ((panel->lcd_ht - panel->lcd_x - panel->lcd_hbp) < 21) {
>>>          dsi_dev[sel]->dsi_tcon_drq.bits.drq_mode = 0;
>>> } else {
>>>          dsi_dev[sel]->dsi_tcon_drq.bits.drq_set =
>>>                  (panel->lcd_ht-panel->lcd_x-panel->lcd_hbp-20) *
>>>                  dsi_pixel_bits[panel->lcd_dsi_format]/(8*4);
>>> }
>>>
>>> It is testing that the sync + front porch is smaller than 21, and
>>> otherwise sets the drq.
>>>
>>>> -     if ((mode->hsync_end - mode->hdisplay) > 20) {
>>>
>>> My code here is testing that the difference between hsync_end and
>>> hdisplay is superior to 20, and sets the DRQ if true. The condition is
>>> reversed, but otherwise, that difference is the front porch plus the
>>> sync length.
>>
>> True, I understand this, but does drq setting here is specific to SoC?
>> I thought of finding DRQ in A31 BSP but I couldn't find the code. do
>> you have bsp somewhere in github?
>>
>>>
>>>> +     if ((mode->hsync_start - mode->hdisplay) > 20) {
>>>
>>> However, you are testing for just the front porch, unlike what your
>>> commit log is saying, and unlike what allwinner's code is saying. So
>>> this deserves some explanation.
>>
>> but A64 is doing this, do you think it's completely A64 specific or
>> testing panel with front porch drq?
> 
> See the above code excerpt:
> panel->lcd_ht - panel->lcd_x - panel->lcd_hbp
> 
> This is hsync + front porch. Not the sole front porch. So no, it's not
> doing this.

=> panel->lcd_ht - panel->lcd_x - panel->lcd_hbp

from drivers/video/sunxi/disp2/disp/de/disp_lcd.c
timmings->hor_front_porch= panel_info->lcd_ht-panel_info->lcd_hbp - 
panel_info->lcd_x;

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

This is simply a front porch.

Re: [PATCH 12/12] arm64: dts: allwinner: bananapi-m64: Bananapi S070WV20-CT16 DSI panel

[Jagan Teki]

On Thu, Sep 27, 2018 at 10:47 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> On Thu, Sep 27, 2018 at 05:18:50PM +0530, Jagan Teki wrote:
> > 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>
> > ---
> >  .../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..c0be949be68d 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,24 @@
> >               serial1 = &uart1;
> >       };
> >
> > +     vdd_bl: regulator@0 {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "bl-3v3";
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +             gpio = <&pio 3 7 GPIO_ACTIVE_HIGH>; /* PD7 */
> > +             enable-active-high;
> > +     };
> > +
> > +     backlight_dsi: backlight-dsi {
> > +             compatible = "pwm-backlight";
> > +             pwms = <&r_pwm 0 50000 PWM_POLARITY_INVERTED>;
> > +             brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
>
> The brightness levels should be increasing linearly from a user point
> of view. Usually, with a PWM backlight, a quadratic rule works much
> better.

Does that mean, 0 40 80 ...

Re: [PATCH 10/12] drm/panel: Add Bananapi S070WV20-CT16 MIPI-DSI panel driver

[Jagan Teki]

On Fri, Sep 28, 2018 at 10:02 AM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Thu, Sep 27, 2018 at 7:50 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Bananapi S070WV20-CT16 is 800x480, 4-lane MIPI-DSI panel which
> > can be used to connect via 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.
>
> As I mentioned before, this PCB uses a ICN6211 MIPI-DSI to RGB bridge.
> Please model it as such.

OK.

>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  drivers/gpu/drm/panel/Kconfig                 |   9 +
> >  drivers/gpu/drm/panel/Makefile                |   1 +
> >  .../gpu/drm/panel/panel-bananapi-s070wv20.c   | 336 ++++++++++++++++++
> >  3 files changed, 346 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panel/panel-bananapi-s070wv20.c
> >
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index 6020c30a33b3..d25960a63b10 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
> > +       tristate "Bananapi S070WV20-CT16 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
> > +         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..345b6475d3a4 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) += panel-bananapi-s070wv20.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.c b/drivers/gpu/drm/panel/panel-bananapi-s070wv20.c
> > new file mode 100644
> > index 000000000000..6c9adbec6dd8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-bananapi-s070wv20.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__} }
> > +
>
> Using the timings from below, and comparing with
>
>    https://shiminblog.github.io/mipi-to-rgb-lcd/
>
> > +static const struct s070wv20_init_cmd s070wv20_init_cmds[] = {
> > +       S070WV20_INIT_CMD(0x7A, 0xC1),
>
> > +       S070WV20_INIT_CMD(0x20, 0x20),
>
> This is likely the lower 8 bits of .hdisplay.
>
> > +       S070WV20_INIT_CMD(0x21, 0xE0),
>
> Lower 8 bits of .vdisplay
>
> > +       S070WV20_INIT_CMD(0x22, 0x13),
>
> Might be ((.hdisplay >> 8) & 0xf | (.vdisplay >> 8) & 0xf) << 4).
>
> > +       S070WV20_INIT_CMD(0x23, 0x28),
>
> HFP, or horizontal front porch. 40 in this case.
>
> > +       S070WV20_INIT_CMD(0x24, 0x30),
>
> HSYNC, or horizontal sync pulse length. 48 in this case.
>
> > +       S070WV20_INIT_CMD(0x25, 0x28),
>
> HBP, or horizontal back porch. 40 in this case.
>
> > +       S070WV20_INIT_CMD(0x26, 0x00),
>
> Unknown. Possibly RGB phase shift?
>
> > +       S070WV20_INIT_CMD(0x27, 0x0D),
>
> VFP, or vertical front porch. 13 in this case.
>
> > +       S070WV20_INIT_CMD(0x28, 0x03),
>
> This might be VSYNC, judging from the grouping. 3 in this case.
>
> > +       S070WV20_INIT_CMD(0x29, 0x1D),
>
> VBP, or vertical back porch. 29 in this case.
>
> > +       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),
>
> This might be related to number of MIPI lanes. I saw 0x88 for 2-lane devices.
>
> > +       S070WV20_INIT_CMD(0xB6, 0x20),
> > +       S070WV20_INIT_CMD(0x51, 0x20),
> > +       S070WV20_INIT_CMD(0x09, 0x10),
>
> The last three seem standardized across all ICN6211 drivers I could
> find on github.
>
> Other drivers / init sequences I found:
>
> https://github.com/yesnoandor/x300/blob/master/kernel/arch/arm/boot/dts/erobbing/x300/x300.dtsi#L81
> https://github.com/wxzed/Raspberry_5MIPI_Display/blob/master/I2C_Slave/USER/main.c#L15
>
> https://github.com/eliot-shao/qcom/blob/master/icn6211_cxn0102/kernel/drivers/video/msm/mdss/mdss_i2c_interface.c#L152
> matches timings for
> https://github.com/eliot-shao/qcom/blob/master/icn6211_cxn0102/kernel/arch/arm/boot/dts/qcom/dsi-mipi-2-rgb_1280p_video.dtsi#L20
>
> https://github.com/zestroly/micromat/blob/master/test/raspberry/ICN6211.cpp#L169
>
> I also reached out to Bananapi to see if they have more information on
> this chip.

Did you get any further information from BPI?

Re: [PATCH 12/12] arm64: dts: allwinner: bananapi-m64: Bananapi S070WV20-CT16 DSI panel

[Maxime Ripard]

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

On Thu, Oct 04, 2018 at 09:33:59PM +0530, Jagan Teki wrote:
> On Thu, Sep 27, 2018 at 10:47 PM Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> >
> > On Thu, Sep 27, 2018 at 05:18:50PM +0530, Jagan Teki wrote:
> > > 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>
> > > ---
> > >  .../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..c0be949be68d 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,24 @@
> > >               serial1 = &uart1;
> > >       };
> > >
> > > +     vdd_bl: regulator@0 {
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "bl-3v3";
> > > +             regulator-min-microvolt = <3300000>;
> > > +             regulator-max-microvolt = <3300000>;
> > > +             gpio = <&pio 3 7 GPIO_ACTIVE_HIGH>; /* PD7 */
> > > +             enable-active-high;
> > > +     };
> > > +
> > > +     backlight_dsi: backlight-dsi {
> > > +             compatible = "pwm-backlight";
> > > +             pwms = <&r_pwm 0 50000 PWM_POLARITY_INVERTED>;
> > > +             brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
> >
> > The brightness levels should be increasing linearly from a user point
> > of view. Usually, with a PWM backlight, a quadratic rule works much
> > better.
> 
> Does that mean, 0 40 80 ...

No, that means 1 2 4 8 16 32 etc..

Maxime

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

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

Re: [linux-sunxi] Re: [PATCH 06/12] drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation

[Maxime Ripard]

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

On Mon, Oct 01, 2018 at 01:39:17PM +0530, Jagan Teki wrote:
> On Saturday 29 September 2018 07:23 PM, Maxime Ripard wrote:
> > On Thu, Sep 27, 2018 at 09:50:34PM +0530, Jagan Teki wrote:
> > > On Thu, Sep 27, 2018 at 8:51 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > 
> > > > On Thu, Sep 27, 2018 at 05:18:44PM +0530, Jagan Teki wrote:
> > > > > According to horizontal and vertical timings are defined
> > > > > per the diagram from include/drm/drm_modes.h
> > > > > 
> > > > > Back porch = [hv]total - [hv]sync_end
> > > > > 
> > > > > So, update SUN6I_DSI_BASIC_SIZE0_VBP calculation as
> > > > > mode->vtotal - mode->vsync_end
> > > > > 
> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > ---
> > > > >   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 1c7e42015645..599284971ab6 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));
> > > > 
> > > > Is it purely theoretical, or did you find some source that back that?
> > > 
> > > VSA is done as per that, sync_end - sync start would give sync time.
> > 
> > That's a different register though.
> > 
> > > VBP also done in BPI-M64-bsp[1] which results back porch existing code
> > > results fron porch.
> > > 
> > > [1] https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L955
> > 
> > That code does back porch - sync length. Such a calculation doesn't
> > make much sense as is, but you're saying that it results in the front
> > porch. Again, what makes you say that?
> 
> No what code shows is not a real back porch value used for drm it's panel
> back porch value which is a DTS property in BSP.
> (I made wrong comment on previous mail as front porch, sorry)
> 
> here is the real code
> 
> from 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;
> 
> 
> u32 vbp = panel->lcd_vbp;
> u32 vspw = panel->lcd_vspw;
> dsi_dev[sel]->dsi_basic_size0.bits.vbp = vbp-vspw;
> 
> So,
> 
> 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
> 
> VSA, which a proper value.
> 
> dsi_dev[sel]->dsi_basic_size0.bits.vsa = vspw;
> => panel_info->lcd_vspw;
> => timmings->ver_sync_time
> => mode->vsync_end - mode->vsync_start

Then please write that 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: 833 bytes --]

Re: [linux-sunxi] Re: [PATCH 07/12] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits

[Maxime Ripard]

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

On Wed, Oct 03, 2018 at 08:52:06AM +0530, Jagan Teki wrote:
> On Tuesday 02 October 2018 06:50 PM, Maxime Ripard wrote:
> > On Thu, Sep 27, 2018 at 11:15:50PM +0530, Jagan Teki wrote:
> > > On Thu, Sep 27, 2018 at 10:28 PM Maxime Ripard
> > > <maxime.ripard@bootlin.com> wrote:
> > > > 
> > > > On Thu, Sep 27, 2018 at 05:18:45PM +0530, Jagan Teki wrote:
> > > > > TCON DRQ set bits for non-burst DSI mode can computed via
> > > > > horizontal front porch instead of front porch + sync timings.
> > > > > 
> > > > > Since there no documentation for TCON_DRQ_REG(0x7c) register
> > > > > this change is taken as reference from BPI-M64-bsp.
> > > > 
> > > > Detailing more what the issue is would be great.
> > > > 
> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > ---
> > > > >   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 599284971ab6..9918fdb990ff 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;
> > > > 
> > > > The computation here is in the A64 driver:
> > > > 
> > > > if ((panel->lcd_ht - panel->lcd_x - panel->lcd_hbp) < 21) {
> > > >          dsi_dev[sel]->dsi_tcon_drq.bits.drq_mode = 0;
> > > > } else {
> > > >          dsi_dev[sel]->dsi_tcon_drq.bits.drq_set =
> > > >                  (panel->lcd_ht-panel->lcd_x-panel->lcd_hbp-20) *
> > > >                  dsi_pixel_bits[panel->lcd_dsi_format]/(8*4);
> > > > }
> > > > 
> > > > It is testing that the sync + front porch is smaller than 21, and
> > > > otherwise sets the drq.
> > > > 
> > > > > -     if ((mode->hsync_end - mode->hdisplay) > 20) {
> > > > 
> > > > My code here is testing that the difference between hsync_end and
> > > > hdisplay is superior to 20, and sets the DRQ if true. The condition is
> > > > reversed, but otherwise, that difference is the front porch plus the
> > > > sync length.
> > > 
> > > True, I understand this, but does drq setting here is specific to SoC?
> > > I thought of finding DRQ in A31 BSP but I couldn't find the code. do
> > > you have bsp somewhere in github?
> > > 
> > > > 
> > > > > +     if ((mode->hsync_start - mode->hdisplay) > 20) {
> > > > 
> > > > However, you are testing for just the front porch, unlike what your
> > > > commit log is saying, and unlike what allwinner's code is saying. So
> > > > this deserves some explanation.
> > > 
> > > but A64 is doing this, do you think it's completely A64 specific or
> > > testing panel with front porch drq?
> > 
> > See the above code excerpt:
> > panel->lcd_ht - panel->lcd_x - panel->lcd_hbp
> > 
> > This is hsync + front porch. Not the sole front porch. So no, it's not
> > doing this.
> 
> => panel->lcd_ht - panel->lcd_x - panel->lcd_hbp
> 
> from drivers/video/sunxi/disp2/disp/de/disp_lcd.c
> timmings->hor_front_porch= panel_info->lcd_ht-panel_info->lcd_hbp -
> panel_info->lcd_x;
> 
> => (timmings->hor_front_porch + panel->lcd_hbp + panel->lcd_x) -
> panel->lcd_x - panel->hbp
> => timmings->hor_front_porch
> => mode->hsync_start - mode->hdisplay
> 
> This is simply a front porch.

And this should be in your commit log as well.

Maxime

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

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

Re: [PATCH 03/12] dt-bindings: sun6i-dsi: Add compatible for A64 MIPI DSI

[Rob Herring]

On Thu, 27 Sep 2018 17:18:41 +0530, Jagan Teki wrote:
> 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>
> ---
>  Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

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

Re: [PATCH 09/12] dt-bindings: panel: Add Bananapi S070WV20-CT16 MIPI-DSI panel bindings

[Rob Herring]

On Thu, 27 Sep 2018 17:18:47 +0530, Jagan Teki wrote:
> Bananapi S070WV20-CT16 is 800x480, 4-lane MIPI-DSI panel, the
> same panel PCB comes with parallel RBG which is supported via
> panel-simple with "bananapi,s070wv20-ct16" compatible.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  .../panel/bananapi,s070wv20-ct16-dsi.txt      | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16-dsi.txt
> 

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

Re: [PATCH 09/12] dt-bindings: panel: Add Bananapi S070WV20-CT16 MIPI-DSI panel bindings

[Chen-Yu Tsai]

Hi Rob,

On Mon, Oct 15, 2018 at 7:24 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, 27 Sep 2018 17:18:47 +0530, Jagan Teki wrote:
> > Bananapi S070WV20-CT16 is 800x480, 4-lane MIPI-DSI panel, the
> > same panel PCB comes with parallel RBG which is supported via
> > panel-simple with "bananapi,s070wv20-ct16" compatible.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  .../panel/bananapi,s070wv20-ct16-dsi.txt      | 21 +++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16-dsi.txt
> >
>
> Reviewed-by: Rob Herring <robh@kernel.org>

This "panel" is actually an RGB panel with a MIPI-DSI-to-RGB bridge
tacked on. On one particular revision of this module, one can also
directly use the RGB interface.

Would it be better to model this as bridge+panel? We already have
a binding for the RGB version [1]. This would make it harder to
make a driver though, as there is no publicly available datasheet
for the bridge chip, so it's likely that part of the init sequence
would have to be hard-coded.

Thanks
ChenYu

[1] Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt

Re: [PATCH 09/12] dt-bindings: panel: Add Bananapi S070WV20-CT16 MIPI-DSI panel bindings

[Rob Herring]

On Mon, Oct 22, 2018 at 5:22 AM Chen-Yu Tsai <wens@csie.org> wrote:
>
> Hi Rob,
>
> On Mon, Oct 15, 2018 at 7:24 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, 27 Sep 2018 17:18:47 +0530, Jagan Teki wrote:
> > > Bananapi S070WV20-CT16 is 800x480, 4-lane MIPI-DSI panel, the
> > > same panel PCB comes with parallel RBG which is supported via
> > > panel-simple with "bananapi,s070wv20-ct16" compatible.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > >  .../panel/bananapi,s070wv20-ct16-dsi.txt      | 21 +++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16-dsi.txt
> > >
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> This "panel" is actually an RGB panel with a MIPI-DSI-to-RGB bridge
> tacked on. On one particular revision of this module, one can also
> directly use the RGB interface.
>
> Would it be better to model this as bridge+panel? We already have
> a binding for the RGB version [1]. This would make it harder to
> make a driver though, as there is no publicly available datasheet
> for the bridge chip, so it's likely that part of the init sequence
> would have to be hard-coded.

Perhaps you can use the same compatible and detect based on the OF
graph connection whether it is DSI or RGB interface? That would mean
the RGB version doesn't use simple-panel driver, but that should be
okay.

If there's other users of this bridge chip, then modeling the bridge
separately would be better.

Rob

Re: [PATCH 09/12] dt-bindings: panel: Add Bananapi S070WV20-CT16 MIPI-DSI panel bindings

[Chen-Yu Tsai]

On Tue, Oct 23, 2018 at 4:11 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Oct 22, 2018 at 5:22 AM Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > Hi Rob,
> >
> > On Mon, Oct 15, 2018 at 7:24 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Thu, 27 Sep 2018 17:18:47 +0530, Jagan Teki wrote:
> > > > Bananapi S070WV20-CT16 is 800x480, 4-lane MIPI-DSI panel, the
> > > > same panel PCB comes with parallel RBG which is supported via
> > > > panel-simple with "bananapi,s070wv20-ct16" compatible.
> > > >
> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > ---
> > > >  .../panel/bananapi,s070wv20-ct16-dsi.txt      | 21 +++++++++++++++++++
> > > >  1 file changed, 21 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16-dsi.txt
> > > >
> > >
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> >
> > This "panel" is actually an RGB panel with a MIPI-DSI-to-RGB bridge
> > tacked on. On one particular revision of this module, one can also
> > directly use the RGB interface.
> >
> > Would it be better to model this as bridge+panel? We already have
> > a binding for the RGB version [1]. This would make it harder to
> > make a driver though, as there is no publicly available datasheet
> > for the bridge chip, so it's likely that part of the init sequence
> > would have to be hard-coded.
>
> Perhaps you can use the same compatible and detect based on the OF
> graph connection whether it is DSI or RGB interface? That would mean
> the RGB version doesn't use simple-panel driver, but that should be
> okay.

I think that would work.

> If there's other users of this bridge chip, then modeling the bridge
> separately would be better.

I doubt we'd find it. And given that there are no docs for the init
sequence, even though we could make out some parts, it will always
be a partially binary sequence tied to the compatible for the panel.

Thanks
ChenYu