[PATCH v2 0/9] drm/sun4i: dsi: Add burst mode support

[PATCH v2 0/9] drm/sun4i: dsi: Add burst mode support

[Maxime Ripard]

Hi,

Here is a series implementing the burst mode support for DSI.

It's been tested on an A33 board with the panel supported on the last
patch, which should remove all quirks due to a different SoC from the
equation.

Let me know what you think,
Maxime

Changes from v2:
  - Change the start delay calculation according to the legacy driver in
    Allwinner's BSP
  - Fixed the edge calculation to add the same parentheses around the
    factors
  - Added a bunch of fixes to timings
  - Added a patch to make hblk computation more accurate, and added a
    comment
  - Renamed the panel to Ronbo and fixed a bunch of things
  - Added the Reviewed-By

Konstantin Sudakov (2):
  drm/sun4i: dsi: Add burst support
  drm/panel: Add Ronbo RB070D30 panel

Maxime Ripard (7):
  fixup! ARM: dts: sun8i-a33: bridgemate: Enable display support
  drm/sun4i: dsi: Restrict DSI tcon clock divider
  drm/sun4i: dsi: Change the start delay calculation
  drm/sun4i: dsi: Enforce boundaries on the start delay
  drm/sun4i: dsi: Fix front vs back porch calculation
  drm/sun4i: dsi: Fix DRQ calculation
  drm/sun4i: dsi: Rework a bit the hblk calculation

 arch/arm/boot/dts/sun8i-a33-bridgemate-bm3.dts |   2 +-
 drivers/gpu/drm/panel/Kconfig                  |   9 +-
 drivers/gpu/drm/panel/Makefile                 |   1 +-
 drivers/gpu/drm/panel/panel-ronbo-rb070d30.c   | 262 ++++++++++++++++++-
 drivers/gpu/drm/sun4i/sun4i_tcon.c             |   4 +-
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c         | 183 ++++++++++---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h         |   2 +-
 7 files changed, 420 insertions(+), 43 deletions(-)
 create mode 100644 drivers/gpu/drm/panel/panel-ronbo-rb070d30.c

base-commit: f6028e7e48b0f3865b0837d400ca783d3d200b62
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/9] fixup! ARM: dts: sun8i-a33: bridgemate: Enable display support

[Maxime Ripard]

---
 arch/arm/boot/dts/sun8i-a33-bridgemate-bm3.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-a33-bridgemate-bm3.dts b/arch/arm/boot/dts/sun8i-a33-bridgemate-bm3.dts
index ea12d12bf221..c26316465dc0 100644
--- a/arch/arm/boot/dts/sun8i-a33-bridgemate-bm3.dts
+++ b/arch/arm/boot/dts/sun8i-a33-bridgemate-bm3.dts
@@ -106,7 +106,7 @@
 	status = "okay";
 
 	panel@0 {
-		compatible = "rondo,rb070d30";
+		compatible = "ronbo,rb070d30";
 		reg = <0>;
 		power-gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>;
 		updn-gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>;
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/9] drm/sun4i: dsi: Restrict DSI tcon clock divider

[Maxime Ripard]

The current code allows the TCON clock divider to have a range between 4
and 127 when feeding the DSI controller.

The only display supported so far had a display clock rate that ended up
using a divider of 4, but testing with other displays show that only 4
seems to be functional.

This also aligns with what Allwinner is doing in their BSP, so let's just
hardcode that we want a divider of 4 when using the DSI output.

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c     | 4 ++--
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 0420f5c978b9..bee73ead732a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -341,8 +341,8 @@ static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon *tcon,
 	u32 block_space, start_delay;
 	u32 tcon_div;
 
-	tcon->dclk_min_div = 4;
-	tcon->dclk_max_div = 127;
+	tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
+	tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
 
 	sun4i_tcon0_mode_set_common(tcon, mode);
 
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index dbbc5b3ecbda..6d4a3c0fd9b5 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -13,6 +13,8 @@
 #include <drm/drm_encoder.h>
 #include <drm/drm_mipi_dsi.h>
 
+#define SUN6I_DSI_TCON_DIV	4
+
 struct sun6i_dphy {
 	struct clk		*bus_clk;
 	struct clk		*mod_clk;
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/9] drm/sun4i: dsi: Change the start delay calculation

[Maxime Ripard]

The current calculation for the video start delay in the current DSI driver
is that it is the total vertical size, minus the front porch and sync length,
plus 1. This equals to the active vertical size plus the back porch plus 1.

That 1 is coming in the Allwinner BSP from an variable that is set to 1.
However, if we look at the Allwinner BSP more closely, and especially in
the "legacy" code for the display (in drivers/video/sunxi/legacy/), we can
see that this variable is actually computed from the porches and the sync
minus 10, clamped between 8 and 100.

This fixes the start delay symptom we've seen on some panels (vblank
timeouts with vertical white stripes at the bottom of the panel).

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 380fc527a707..9471fa695ec7 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -357,7 +357,9 @@ 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;
+	u16 start = clamp(mode->vtotal - mode->vdisplay - 10, 8, 100);
+
+	return mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;
 }
 
 static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/9] drm/sun4i: dsi: Enforce boundaries on the start delay

[Maxime Ripard]

The Allwinner BSP makes sure that we don't end up with a null start delay
or with a delay larger than vtotal.

The former condition is likely to happen now with the reworked start delay,
so make sure we enforce the same boundaries.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 9471fa695ec7..506f2e8cf454 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -358,8 +358,12 @@ static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi,
 					   struct drm_display_mode *mode)
 {
 	u16 start = clamp(mode->vtotal - mode->vdisplay - 10, 8, 100);
+	u16 delay = mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;
 
-	return mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;
+	if (delay > mode->vtotal)
+		delay = delay % mode->vtotal;
+
+	return max_t(u16, delay, 1);
 }
 
 static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 5/9] drm/sun4i: dsi: Fix front vs back porch calculation

[Maxime Ripard]

Since I always confuse the back and front porches, a few miscalculation
slipped through. Fix them.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 506f2e8cf454..2518a0d7567c 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -477,7 +477,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	 */
 #define HBP_PACKET_OVERHEAD	6
 	hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
-		  (mode->hsync_start - mode->hdisplay) * Bpp - HBP_PACKET_OVERHEAD);
+		  (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
 
 	/*
 	 * The frontporch is set using a blanking packet (4 bytes +
@@ -485,7 +485,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	 */
 #define HFP_PACKET_OVERHEAD	6
 	hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
-		  (mode->htotal - mode->hsync_end) * Bpp - HFP_PACKET_OVERHEAD);
+		  (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
 
 	/*
 	 * hblk seems to be the line + porches length.
@@ -531,8 +531,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) |
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 6/9] drm/sun4i: dsi: Fix DRQ calculation

[Maxime Ripard]

The DRQ calculation code in the Allwinner BSP uses the vertical front
porch value as the condition, but we're using the video back porch.

Since I always confuse the two, and I'm the original author of that code, I
guess I deserved a brown paper bag.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.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 2518a0d7567c..8e6392831e9d 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -372,9 +372,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;
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 7/9] drm/sun4i: dsi: Rework a bit the hblk calculation

[Maxime Ripard]

It turns out that the hblk calculation actually follows a similar pattern
than the other packets. Rework a bit the calculation and add a comment.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 8e6392831e9d..e0288e7dc64e 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -488,9 +488,13 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 		  (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
 
 	/*
-	 * hblk seems to be the line + porches length.
+	 * The blanking is set using a sync event (4 bytes) and a
+	 * blanking packet (4 bytes + payload + 2 bytes). Its minimal
+	 * size is therefore 10 bytes.
 	 */
-	hblk = mode->htotal * Bpp - hsa;
+#define HBLK_PACKET_OVERHEAD	10
+	hblk = max((unsigned int)HBLK_PACKET_OVERHEAD,
+		   (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp - HBLK_PACKET_OVERHEAD);
 
 	/*
 	 * And I'm not entirely sure what vblk is about. The driver in
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 8/9] drm/sun4i: dsi: Add burst support

[Maxime Ripard]

From: Konstantin Sudakov <k.sudakov@integrasources.com>

The current driver doesn't support the DSI burst operation mode.

Let's add the needed quirks to make it work.

Signed-off-by: Konstantin Sudakov <k.sudakov@integrasources.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 171 ++++++++++++++++++++------
 1 file changed, 132 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index e0288e7dc64e..4cb715dc9100 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -23,7 +23,9 @@
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_panel.h>
 
+#include "sun4i_crtc.h"
 #include "sun4i_drv.h"
+#include "sun4i_tcon.h"
 #include "sun6i_mipi_dsi.h"
 
 #include <video/mipi_display.h>
@@ -32,6 +34,8 @@
 #define SUN6I_DSI_CTL_EN			BIT(0)
 
 #define SUN6I_DSI_BASIC_CTL_REG		0x00c
+#define SUN6I_DSI_BASIC_CTL_TRAIL_INV(n)		(((n) & 0xf) << 4)
+#define SUN6I_DSI_BASIC_CTL_TRAIL_FILL		BIT(3)
 #define SUN6I_DSI_BASIC_CTL_HBP_DIS		BIT(2)
 #define SUN6I_DSI_BASIC_CTL_HSA_HSE_DIS		BIT(1)
 #define SUN6I_DSI_BASIC_CTL_VIDEO_BURST		BIT(0)
@@ -152,6 +156,8 @@
 
 #define SUN6I_DSI_CMD_TX_REG(n)		(0x300 + (n) * 0x04)
 
+#define SUN6I_DSI_SYNC_POINT		40
+
 enum sun6i_dsi_start_inst {
 	DSI_START_LPRX,
 	DSI_START_LPTX,
@@ -366,13 +372,70 @@ static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi,
 	return max_t(u16, delay, 1);
 }
 
+static u16 sun6i_dsi_get_line_num(struct sun6i_dsi *dsi,
+				  struct drm_display_mode *mode)
+{
+	struct mipi_dsi_device *device = dsi->device;
+	unsigned Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
+
+	return mode->htotal * Bpp / device->lanes;
+}
+
+static u16 sun6i_dsi_get_drq_edge0(struct sun6i_dsi *dsi,
+				   struct drm_display_mode *mode,
+				   u16 line_num, u16 edge1)
+{
+	u16 edge0 = edge1;
+
+	edge0 += (mode->hdisplay + 40) * SUN6I_DSI_TCON_DIV / 8;
+
+	if (edge0 > line_num)
+		return edge0 - line_num;
+
+	return 1;
+}
+
+static u16 sun6i_dsi_get_drq_edge1(struct sun6i_dsi *dsi,
+				   struct drm_display_mode *mode,
+				   u16 line_num)
+{
+	struct mipi_dsi_device *device = dsi->device;
+	unsigned Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
+	unsigned hbp = mode->htotal - mode->hsync_end;
+	u16 edge1;
+
+	edge1 = SUN6I_DSI_SYNC_POINT;
+	edge1 += (mode->hdisplay + hbp + 20) * Bpp / device->lanes;
+
+	if (edge1 > line_num)
+		return line_num;
+
+	return edge1;
+}
+
 static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
 				  struct drm_display_mode *mode)
 {
 	struct mipi_dsi_device *device = dsi->device;
 	u32 val = 0;
 
-	if ((mode->hsync_start - mode->hdisplay) > 20) {
+	if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
+		u16 line_num = sun6i_dsi_get_line_num(dsi, mode);
+		u16 edge0, edge1;
+
+		edge1 = sun6i_dsi_get_drq_edge1(dsi, mode, line_num);
+		edge0 = sun6i_dsi_get_drq_edge0(dsi, mode, line_num, edge1);
+
+		regmap_write(dsi->regs, SUN6I_DSI_BURST_DRQ_REG,
+			     SUN6I_DSI_BURST_DRQ_EDGE0(edge0) |
+			     SUN6I_DSI_BURST_DRQ_EDGE1(edge1));
+
+		regmap_write(dsi->regs, SUN6I_DSI_BURST_LINE_REG,
+			     SUN6I_DSI_BURST_LINE_NUM(line_num) |
+			     SUN6I_DSI_BURST_LINE_SYNC_POINT(SUN6I_DSI_SYNC_POINT));
+
+		val = SUN6I_DSI_TCON_DRQ_ENABLE_MODE;
+	} else if ((mode->hsync_start - mode->hdisplay) > 20) {
 		/* Maaaaaagic */
 		u16 drq = (mode->hsync_start - mode->hdisplay) - 20;
 
@@ -389,8 +452,19 @@ static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
 static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi *dsi,
 				      struct drm_display_mode *mode)
 {
+	struct mipi_dsi_device *device = dsi->device;
 	u16 delay = 50 - 1;
 
+	if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
+		delay = (mode->htotal - mode->hdisplay) * 150;
+		delay /= (mode->clock / 1000) * 8;
+		delay -= 50;
+	}
+
+	regmap_write(dsi->regs, SUN6I_DSI_INST_LOOP_SEL_REG,
+			 2 << (4 * DSI_INST_ID_LP11) |
+			 3 << (4 * DSI_INST_ID_DLY));
+
 	regmap_write(dsi->regs, SUN6I_DSI_INST_LOOP_NUM_REG(0),
 		     SUN6I_DSI_INST_LOOP_NUM_N0(50 - 1) |
 		     SUN6I_DSI_INST_LOOP_NUM_N1(delay));
@@ -457,52 +531,71 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	struct mipi_dsi_device *device = dsi->device;
 	unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
 	u16 hbp, hfp, hsa, hblk, vblk;
+	u32 basic_ctl = 0;
 	size_t bytes;
 	u8 *buffer;
 
 	/* Do all timing calculations up front to allocate buffer space */
 
-	/*
-	 * A sync period is composed of a blanking packet (4 bytes +
-	 * payload + 2 bytes) and a sync event packet (4 bytes). Its
-	 * minimal size is therefore 10 bytes
-	 */
+	if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
+		hsa = 0;
+		hbp = 0;
+		hfp = 0;
+		hblk = mode->hdisplay * Bpp;
+		vblk = 0;
+		basic_ctl = SUN6I_DSI_BASIC_CTL_VIDEO_BURST |
+			    SUN6I_DSI_BASIC_CTL_HSA_HSE_DIS |
+			    SUN6I_DSI_BASIC_CTL_HBP_DIS;
+
+		if (device->lanes == 4)
+			basic_ctl |= SUN6I_DSI_BASIC_CTL_TRAIL_FILL |
+				     SUN6I_DSI_BASIC_CTL_TRAIL_INV(0xc);
+	} else {
+		/*
+		 * A sync period is composed of a blanking packet (4
+		 * bytes + payload + 2 bytes) and a sync event packet
+		 * (4 bytes). Its minimal size is therefore 10 bytes
+		 */
 #define HSA_PACKET_OVERHEAD	10
-	hsa = max((unsigned int)HSA_PACKET_OVERHEAD,
-		  (mode->hsync_end - mode->hsync_start) * Bpp - HSA_PACKET_OVERHEAD);
-
-	/*
-	 * The backporch is set using a blanking packet (4 bytes +
-	 * payload + 2 bytes). Its minimal size is therefore 6 bytes
-	 */
+		hsa = max((unsigned int)HSA_PACKET_OVERHEAD,
+			  (mode->hsync_end - mode->hsync_start) * Bpp - HSA_PACKET_OVERHEAD);
+
+		/*
+		 * The backporch is set using a blanking packet (4
+		 * bytes + payload + 2 bytes). Its minimal size is
+		 * therefore 6 bytes
+		 */
 #define HBP_PACKET_OVERHEAD	6
-	hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
-		  (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
-
-	/*
-	 * The frontporch is set using a blanking packet (4 bytes +
-	 * payload + 2 bytes). Its minimal size is therefore 6 bytes
-	 */
+		hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
+			  (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
+
+		/*
+		 * The frontporch is set using a blanking packet (4
+		 * bytes + payload + 2 bytes). Its minimal size is
+		 * therefore 6 bytes
+		 */
 #define HFP_PACKET_OVERHEAD	6
-	hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
-		  (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
-
-	/*
-	 * The blanking is set using a sync event (4 bytes) and a
-	 * blanking packet (4 bytes + payload + 2 bytes). Its minimal
-	 * size is therefore 10 bytes.
-	 */
+		hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
+			  (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
+
+		/*
+		 * The blanking is set using a sync event (4 bytes)
+		 * and a blanking packet (4 bytes + payload + 2
+		 * bytes). Its minimal size is therefore 10 bytes.
+		 */
 #define HBLK_PACKET_OVERHEAD	10
-	hblk = max((unsigned int)HBLK_PACKET_OVERHEAD,
-		   (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp - HBLK_PACKET_OVERHEAD);
-
-	/*
-	 * And I'm not entirely sure what vblk is about. The driver in
-	 * Allwinner BSP is using a rather convoluted calculation
-	 * there only for 4 lanes. However, using 0 (the !4 lanes
-	 * case) even with a 4 lanes screen seems to work...
-	 */
-	vblk = 0;
+		hblk = max((unsigned int)HBLK_PACKET_OVERHEAD,
+			   (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp -
+			   HBLK_PACKET_OVERHEAD);
+
+		/*
+		 * And I'm not entirely sure what vblk is about. The driver in
+		 * Allwinner BSP is using a rather convoluted calculation
+		 * there only for 4 lanes. However, using 0 (the !4 lanes
+		 * case) even with a 4 lanes screen seems to work...
+		 */
+		vblk = 0;
+	}
 
 	/* How many bytes do we need to send all payloads? */
 	bytes = max_t(size_t, max(max(hfp, hblk), max(hsa, hbp)), vblk);
@@ -510,7 +603,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
 	if (WARN_ON(!buffer))
 		return;
 
-	regmap_write(dsi->regs, SUN6I_DSI_BASIC_CTL_REG, 0);
+	regmap_write(dsi->regs, SUN6I_DSI_BASIC_CTL_REG, basic_ctl);
 
 	regmap_write(dsi->regs, SUN6I_DSI_SYNC_HSS_REG,
 		     sun6i_dsi_build_sync_pkt(MIPI_DSI_H_SYNC_START,
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 9/9] drm/panel: Add Ronbo RB070D30 panel

[Maxime Ripard]

From: Konstantin Sudakov <k.sudakov@integrasources.com>

The Ronbo RB070D30 panel is a MIPI-DSI panel based on a Fitipower EK79007
controller and a 1024x600 panel.

Signed-off-by: Konstantin Sudakov <k.sudakov@integrasources.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/panel/Kconfig                |   9 +-
 drivers/gpu/drm/panel/Makefile               |   1 +-
 drivers/gpu/drm/panel/panel-ronbo-rb070d30.c | 262 ++++++++++++++++++++-
 3 files changed, 272 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-ronbo-rb070d30.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 3f3537719beb..870dcdbf2601 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -138,6 +138,15 @@ config DRM_PANEL_RAYDIUM_RM68200
 	  Say Y here if you want to enable support for Raydium RM68200
 	  720x1280 DSI video mode panel.
 
+config DRM_PANEL_RONBO_RB070D30
+	tristate "Ronbo Electronics RB070D30 panel"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y here if you want to enable support for Ronbo Electronics
+	  RB070D30 1024x600 DSI panel.
+
 config DRM_PANEL_SAMSUNG_S6D16D0
 	tristate "Samsung S6D16D0 DSI video mode panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 4396658a7996..4a103d346809 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o
 obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o
 obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen.o
 obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
+obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
diff --git a/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c b/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
new file mode 100644
index 000000000000..0fd67bc49927
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018-2019, Bridge Systems BV
+ * Copyright (C) 2018-2019, Bootlin
+ * Copyright (C) 2017, Free Electrons
+ *
+ * This file based on panel-ilitek-ili9881c.c
+ */
+
+#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/media-bus-format.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 <drm/drm_print.h>
+
+#include <video/mipi_display.h>
+#include <video/of_display_timing.h>
+#include <video/videomode.h>
+
+struct rb070d30_panel {
+	struct drm_panel panel;
+	struct mipi_dsi_device *dsi;
+	struct backlight_device *backlight;
+	struct regulator *supply;
+
+	struct {
+		struct gpio_desc *power;
+		struct gpio_desc *reset;
+		struct gpio_desc *updn;
+		struct gpio_desc *shlr;
+	} gpios;
+};
+
+static inline struct rb070d30_panel *panel_to_rb070d30_panel(struct drm_panel *panel)
+{
+	return container_of(panel, struct rb070d30_panel, panel);
+}
+
+static int rb070d30_panel_prepare(struct drm_panel *panel)
+{
+	struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+	int ret;
+
+	ret = regulator_enable(ctx->supply);
+	if (ret < 0) {
+		DRM_DEV_ERROR(&ctx->dsi->dev, "Failed to enable supply: %d\n", ret);
+		return ret;
+	}
+
+	/* Reset */
+	msleep(20);
+	gpiod_set_value(ctx->gpios.power, 1);
+	msleep(20);
+	gpiod_set_value(ctx->gpios.reset, 1);
+	msleep(20);
+	return 0;
+}
+
+static int rb070d30_panel_unprepare(struct drm_panel *panel)
+{
+	struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+
+	gpiod_set_value(ctx->gpios.reset, 0);
+	gpiod_set_value(ctx->gpios.power, 0);
+	regulator_disable(ctx->supply);
+
+	return 0;
+}
+
+static int rb070d30_panel_enable(struct drm_panel *panel)
+{
+	struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+	int ret;
+
+	ret = mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
+	if (ret)
+		return ret;
+
+	ret = backlight_enable(ctx->backlight);
+	if (ret)
+		goto out;
+
+	return 0;
+
+out:
+	mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+	return ret;
+}
+
+static int rb070d30_panel_disable(struct drm_panel *panel)
+{
+	struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+
+	backlight_disable(ctx->backlight);
+	return mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+}
+
+/* Default timings */
+static const struct drm_display_mode default_mode = {
+	.clock		= 51206,
+	.hdisplay	= 1024,
+	.hsync_start	= 1024 + 160,
+	.hsync_end	= 1024 + 160 + 80,
+	.htotal		= 1024 + 160 + 80 + 80,
+	.vdisplay	= 600,
+	.vsync_start	= 600 + 12,
+	.vsync_end	= 600 + 12 + 10,
+	.vtotal		= 600 + 12 + 10 + 13,
+	.vrefresh	= 60,
+
+	.width_mm	= 154,
+	.height_mm	= 85,
+};
+
+static int rb070d30_panel_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+
+	mode = drm_mode_duplicate(panel->drm, &default_mode);
+	if (!mode) {
+		DRM_DEV_ERROR(&ctx->dsi->dev,
+			      "Failed to add mode " DRM_MODE_FMT "\n",
+			      DRM_MODE_ARG(mode));
+		return -EINVAL;
+	}
+
+	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.bpc = 8;
+	panel->connector->display_info.width_mm = mode->width_mm;
+	panel->connector->display_info.height_mm = mode->height_mm;
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 &bus_format, 1);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs rb070d30_panel_funcs = {
+	.get_modes	= rb070d30_panel_get_modes,
+	.prepare	= rb070d30_panel_prepare,
+	.enable		= rb070d30_panel_enable,
+	.disable	= rb070d30_panel_disable,
+	.unprepare	= rb070d30_panel_unprepare,
+};
+
+static int rb070d30_panel_dsi_probe(struct mipi_dsi_device *dsi)
+{
+	struct rb070d30_panel *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->supply = devm_regulator_get(&dsi->dev, "vcc-lcd");
+	if (IS_ERR(ctx->supply))
+		return PTR_ERR(ctx->supply);
+
+	mipi_dsi_set_drvdata(dsi, ctx);
+	ctx->dsi = dsi;
+
+	drm_panel_init(&ctx->panel);
+	ctx->panel.dev = &dsi->dev;
+	ctx->panel.funcs = &rb070d30_panel_funcs;
+
+	ctx->gpios.reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->gpios.reset)) {
+		DRM_DEV_ERROR(&dsi->dev, "Couldn't get our reset GPIO\n");
+		return PTR_ERR(ctx->gpios.reset);
+	}
+
+	ctx->gpios.power = devm_gpiod_get(&dsi->dev, "power", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->gpios.power)) {
+		DRM_DEV_ERROR(&dsi->dev, "Couldn't get our power GPIO\n");
+		return PTR_ERR(ctx->gpios.power);
+	}
+
+	/*
+	 * We don't change the state of that GPIO later on but we need
+	 * to force it into a low state.
+	 */
+	ctx->gpios.updn = devm_gpiod_get(&dsi->dev, "updn", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->gpios.updn)) {
+		DRM_DEV_ERROR(&dsi->dev, "Couldn't get our updn GPIO\n");
+		return PTR_ERR(ctx->gpios.updn);
+	}
+
+	/*
+	 * We don't change the state of that GPIO later on but we need
+	 * to force it into a low state.
+	 */
+	ctx->gpios.shlr = devm_gpiod_get(&dsi->dev, "shlr", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->gpios.shlr)) {
+		DRM_DEV_ERROR(&dsi->dev, "Couldn't get our shlr GPIO\n");
+		return PTR_ERR(ctx->gpios.shlr);
+	}
+
+	ctx->backlight = devm_of_find_backlight(&dsi->dev);
+	if (IS_ERR(ctx->backlight)) {
+		DRM_DEV_ERROR(&dsi->dev, "Couldn't get our backlight\n");
+		return PTR_ERR(ctx->backlight);
+	}
+
+	ret = drm_panel_add(&ctx->panel);
+	if (ret < 0)
+		return ret;
+
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_MODE_LPM;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->lanes = 4;
+
+	return mipi_dsi_attach(dsi);
+}
+
+static int rb070d30_panel_dsi_remove(struct mipi_dsi_device *dsi)
+{
+	struct rb070d30_panel *ctx = mipi_dsi_get_drvdata(dsi);
+
+	mipi_dsi_detach(dsi);
+	drm_panel_remove(&ctx->panel);
+
+	return 0;
+}
+
+static const struct of_device_id rb070d30_panel_of_match[] = {
+	{ .compatible = "ronbo,rb070d30" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, rb070d30_panel_of_match);
+
+static struct mipi_dsi_driver rb070d30_panel_driver = {
+	.probe = rb070d30_panel_dsi_probe,
+	.remove = rb070d30_panel_dsi_remove,
+	.driver = {
+		.name = "panel-ronbo-rb070d30",
+		.of_match_table	= rb070d30_panel_of_match,
+	},
+};
+module_mipi_dsi_driver(rb070d30_panel_driver);
+
+MODULE_AUTHOR("Boris Brezillon <boris.brezillon@bootlin.com>");
+MODULE_AUTHOR("Konstantin Sudakov <k.sudakov@integrasources.com>");
+MODULE_DESCRIPTION("Ronbo RB070D30 Panel Driver");
+MODULE_LICENSE("GPL");
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/9] fixup! ARM: dts: sun8i-a33: bridgemate: Enable display support

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 328 bytes --]

On Fri, Feb 08, 2019 at 10:53:14AM +0100, Maxime Ripard wrote:
> ---
>  arch/arm/boot/dts/sun8i-a33-bridgemate-bm3.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Erm, that one should be ignored obviously...

Maxime

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

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 9/9] drm/panel: Add Ronbo RB070D30 panel

[Konstantin Sudakov]

Hello, Maxime!

>+mode = drm_mode_duplicate(panel->drm, &default_mode);
>+if (!mode) {
>+DRM_DEV_ERROR(&ctx->dsi->dev,
>+      "Failed to add mode " DRM_MODE_FMT "\n",
>+      DRM_MODE_ARG(mode));
>+return -EINVAL;
>+}

Just small fix: DRM_MODE_ARG(mode) -> DRM_MODE_ARG(&default_mode).
The "mode" variable is NULL, it leads to the UB.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 9/9] drm/panel: Add Ronbo RB070D30 panel

[Sam Ravnborg]

Hi Maxime,

On Fri, Feb 08, 2019 at 10:53:22AM +0100, Maxime Ripard wrote:
> From: Konstantin Sudakov <k.sudakov@integrasources.com>
> 
> The Ronbo RB070D30 panel is a MIPI-DSI panel based on a Fitipower EK79007
> controller and a 1024x600 panel.
> 
> Signed-off-by: Konstantin Sudakov <k.sudakov@integrasources.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

Driver looks good, no further comments.

With the issue pointed out by Konstantin you can add my:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 9/9] drm/panel: Add Ronbo RB070D30 panel

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 570 bytes --]

hi,

On Fri, Feb 08, 2019 at 03:24:12PM +0300, Konstantin Sudakov wrote:
> Hello, Maxime!
> 
> >+mode = drm_mode_duplicate(panel->drm, &default_mode);
> >+if (!mode) {
> >+DRM_DEV_ERROR(&ctx->dsi->dev,
> >+      "Failed to add mode " DRM_MODE_FMT "\n",
> >+      DRM_MODE_ARG(mode));
> >+return -EINVAL;
> >+}
> 
> Just small fix: DRM_MODE_ARG(mode) -> DRM_MODE_ARG(&default_mode).
> The "mode" variable is NULL, it leads to the UB.

Good catch, thanks!
Maxime

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

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/9] drm/sun4i: dsi: Change the start delay calculation

[Jagan Teki]

Hi Maxime,

On Fri, Feb 8, 2019 at 3:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> The current calculation for the video start delay in the current DSI driver
> is that it is the total vertical size, minus the front porch and sync length,
> plus 1. This equals to the active vertical size plus the back porch plus 1.
>
> That 1 is coming in the Allwinner BSP from an variable that is set to 1.
> However, if we look at the Allwinner BSP more closely, and especially in
> the "legacy" code for the display (in drivers/video/sunxi/legacy/), we can
> see that this variable is actually computed from the porches and the sync
> minus 10, clamped between 8 and 100.
>
> This fixes the start delay symptom we've seen on some panels (vblank
> timeouts with vertical white stripes at the bottom of the panel).
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 380fc527a707..9471fa695ec7 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -357,7 +357,9 @@ 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;
> +       u16 start = clamp(mode->vtotal - mode->vdisplay - 10, 8, 100);
> +
> +       return mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;
>  }

As you stated in commit message about result as "active vertical size
+ Back porch" which indeed correct in reverse way of previous code.
but the change seems incorrect to me.

=> mode->vtotal - (mode->vsync_end - mode->vdisplay) + start
=> mode->vtotal - (mode->vsync_end - mode->vdisplay) + (mode->vtotal -
mode->vdisplay) # bypass 10 for now
=> mode->vtotal - mode->vsync_end +  mode->vdisplay + mode->vtotal -
mode->vdisplay
=> (mode->vtotal - mode->vsync_end) + mode->vtotal
=> Back porch + mode->vtotal

but not
=> Back porch + mode->vdisplay

Let me know if I'm wrong?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/9] drm/sun4i: dsi: Change the start delay calculation

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2580 bytes --]

On Thu, Mar 07, 2019 at 11:24:01PM +0530, Jagan Teki wrote:
> Hi Maxime,
> 
> On Fri, Feb 8, 2019 at 3:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > The current calculation for the video start delay in the current DSI driver
> > is that it is the total vertical size, minus the front porch and sync length,
> > plus 1. This equals to the active vertical size plus the back porch plus 1.
> >
> > That 1 is coming in the Allwinner BSP from an variable that is set to 1.
> > However, if we look at the Allwinner BSP more closely, and especially in
> > the "legacy" code for the display (in drivers/video/sunxi/legacy/), we can
> > see that this variable is actually computed from the porches and the sync
> > minus 10, clamped between 8 and 100.
> >
> > This fixes the start delay symptom we've seen on some panels (vblank
> > timeouts with vertical white stripes at the bottom of the panel).
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 380fc527a707..9471fa695ec7 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -357,7 +357,9 @@ 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;
> > +       u16 start = clamp(mode->vtotal - mode->vdisplay - 10, 8, 100);
> > +
> > +       return mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;
> >  }
> 
> As you stated in commit message about result as "active vertical size
> + Back porch" which indeed correct in reverse way of previous code.
> but the change seems incorrect to me.
>
> => mode->vtotal - (mode->vsync_end - mode->vdisplay) + start
> => mode->vtotal - (mode->vsync_end - mode->vdisplay) + (mode->vtotal -
> mode->vdisplay) # bypass 10 for now
> => mode->vtotal - mode->vsync_end +  mode->vdisplay + mode->vtotal -
> mode->vdisplay
> => (mode->vtotal - mode->vsync_end) + mode->vtotal
> => Back porch + mode->vtotal
> 
> but not
> => Back porch + mode->vdisplay

Why do you think it should be this result?

Maxime

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

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/9] drm/sun4i: dsi: Change the start delay calculation

[Jagan Teki]

On Mon, Mar 11, 2019 at 7:42 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Thu, Mar 07, 2019 at 11:24:01PM +0530, Jagan Teki wrote:
> > Hi Maxime,
> >
> > On Fri, Feb 8, 2019 at 3:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > The current calculation for the video start delay in the current DSI driver
> > > is that it is the total vertical size, minus the front porch and sync length,
> > > plus 1. This equals to the active vertical size plus the back porch plus 1.
> > >
> > > That 1 is coming in the Allwinner BSP from an variable that is set to 1.
> > > However, if we look at the Allwinner BSP more closely, and especially in
> > > the "legacy" code for the display (in drivers/video/sunxi/legacy/), we can
> > > see that this variable is actually computed from the porches and the sync
> > > minus 10, clamped between 8 and 100.
> > >
> > > This fixes the start delay symptom we've seen on some panels (vblank
> > > timeouts with vertical white stripes at the bottom of the panel).
> > >
> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > ---
> > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > index 380fc527a707..9471fa695ec7 100644
> > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > @@ -357,7 +357,9 @@ 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;
> > > +       u16 start = clamp(mode->vtotal - mode->vdisplay - 10, 8, 100);
> > > +
> > > +       return mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;
> > >  }
> >
> > As you stated in commit message about result as "active vertical size
> > + Back porch" which indeed correct in reverse way of previous code.
> > but the change seems incorrect to me.
> >
> > => mode->vtotal - (mode->vsync_end - mode->vdisplay) + start
> > => mode->vtotal - (mode->vsync_end - mode->vdisplay) + (mode->vtotal -
> > mode->vdisplay) # bypass 10 for now
> > => mode->vtotal - mode->vsync_end +  mode->vdisplay + mode->vtotal -
> > mode->vdisplay
> > => (mode->vtotal - mode->vsync_end) + mode->vtotal
> > => Back porch + mode->vtotal
> >
> > but not
> > => Back porch + mode->vdisplay
>
> Why do you think it should be this result?

This is what the commit messages mentioned. which indeed true.

"The current calculation for the video start delay in the current DSI driver
is that it is the total vertical size, minus the front porch and sync length,
plus 1. This equals to the active vertical size plus the back porch plus 1."

But the result of the logic is
=> Back porch + mode->vtotal

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/9] drm/sun4i: dsi: Change the start delay calculation

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3417 bytes --]

On Mon, Mar 11, 2019 at 08:07:29PM +0530, Jagan Teki wrote:
> On Mon, Mar 11, 2019 at 7:42 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Thu, Mar 07, 2019 at 11:24:01PM +0530, Jagan Teki wrote:
> > > Hi Maxime,
> > >
> > > On Fri, Feb 8, 2019 at 3:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > The current calculation for the video start delay in the current DSI driver
> > > > is that it is the total vertical size, minus the front porch and sync length,
> > > > plus 1. This equals to the active vertical size plus the back porch plus 1.
> > > >
> > > > That 1 is coming in the Allwinner BSP from an variable that is set to 1.
> > > > However, if we look at the Allwinner BSP more closely, and especially in
> > > > the "legacy" code for the display (in drivers/video/sunxi/legacy/), we can
> > > > see that this variable is actually computed from the porches and the sync
> > > > minus 10, clamped between 8 and 100.
> > > >
> > > > This fixes the start delay symptom we've seen on some panels (vblank
> > > > timeouts with vertical white stripes at the bottom of the panel).
> > > >
> > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > > ---
> > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > index 380fc527a707..9471fa695ec7 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > @@ -357,7 +357,9 @@ 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;
> > > > +       u16 start = clamp(mode->vtotal - mode->vdisplay - 10, 8, 100);
> > > > +
> > > > +       return mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;
> > > >  }
> > >
> > > As you stated in commit message about result as "active vertical size
> > > + Back porch" which indeed correct in reverse way of previous code.
> > > but the change seems incorrect to me.
> > >
> > > => mode->vtotal - (mode->vsync_end - mode->vdisplay) + start
> > > => mode->vtotal - (mode->vsync_end - mode->vdisplay) + (mode->vtotal -
> > > mode->vdisplay) # bypass 10 for now
> > > => mode->vtotal - mode->vsync_end +  mode->vdisplay + mode->vtotal -
> > > mode->vdisplay
> > > => (mode->vtotal - mode->vsync_end) + mode->vtotal
> > > => Back porch + mode->vtotal
> > >
> > > but not
> > > => Back porch + mode->vdisplay
> >
> > Why do you think it should be this result?
>
> This is what the commit messages mentioned. which indeed true.
>
> "The current calculation for the video start delay in the current DSI driver
> is that it is the total vertical size, minus the front porch and sync length,
> plus 1. This equals to the active vertical size plus the back porch plus 1."
>
> But the result of the logic is
> => Back porch + mode->vtotal

Again, that commit was assuming that the back porch contained the sync
period, which doesn't seem to be the case. So I'll ask again, what
makes you say this isn't true?

Maxime

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

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel