linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support
@ 2019-02-11 14:41 Maxime Ripard
  2019-02-11 14:41 ` [PATCH v3 1/8] drm/sun4i: dsi: Restrict DSI tcon clock divider Maxime Ripard
                   ` (9 more replies)
  0 siblings, 10 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-02-11 14:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Sean Paul, Maxime Ripard
  Cc: bbrezillon, dri-devel, Paul Kocialkowski, Chen-Yu Tsai,
	Thomas Petazzoni, Jagan Teki, linux-arm-kernel

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 v3:
  - Fixed error in Ronbo panel error path

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 (6):
  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

 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 +-
 6 files changed, 419 insertions(+), 42 deletions(-)
 create mode 100644 drivers/gpu/drm/panel/panel-ronbo-rb070d30.c

base-commit: 7e571a7f2fc622f81f8be4a4d8725551490378b2
-- 
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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v3 1/8] drm/sun4i: dsi: Restrict DSI tcon clock divider
  2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
@ 2019-02-11 14:41 ` Maxime Ripard
  2019-02-11 14:41 ` [PATCH v3 2/8] drm/sun4i: dsi: Change the start delay calculation Maxime Ripard
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-02-11 14:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Sean Paul, Maxime Ripard
  Cc: bbrezillon, dri-devel, Paul Kocialkowski, Chen-Yu Tsai,
	Thomas Petazzoni, Jagan Teki, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 2/8] drm/sun4i: dsi: Change the start delay calculation
  2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
  2019-02-11 14:41 ` [PATCH v3 1/8] drm/sun4i: dsi: Restrict DSI tcon clock divider Maxime Ripard
@ 2019-02-11 14:41 ` Maxime Ripard
  2019-02-12 15:28   ` Paul Kocialkowski
  2019-02-11 14:41 ` [PATCH v3 3/8] drm/sun4i: dsi: Enforce boundaries on the start delay Maxime Ripard
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-02-11 14:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Sean Paul, Maxime Ripard
  Cc: bbrezillon, dri-devel, Paul Kocialkowski, Chen-Yu Tsai,
	Thomas Petazzoni, Jagan Teki, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 3/8] drm/sun4i: dsi: Enforce boundaries on the start delay
  2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
  2019-02-11 14:41 ` [PATCH v3 1/8] drm/sun4i: dsi: Restrict DSI tcon clock divider Maxime Ripard
  2019-02-11 14:41 ` [PATCH v3 2/8] drm/sun4i: dsi: Change the start delay calculation Maxime Ripard
@ 2019-02-11 14:41 ` Maxime Ripard
  2019-02-12 15:30   ` Paul Kocialkowski
  2019-02-11 14:41 ` [PATCH v3 4/8] drm/sun4i: dsi: Fix front vs back porch calculation Maxime Ripard
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-02-11 14:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Sean Paul, Maxime Ripard
  Cc: bbrezillon, dri-devel, Paul Kocialkowski, Chen-Yu Tsai,
	Thomas Petazzoni, Jagan Teki, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 4/8] drm/sun4i: dsi: Fix front vs back porch calculation
  2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
                   ` (2 preceding siblings ...)
  2019-02-11 14:41 ` [PATCH v3 3/8] drm/sun4i: dsi: Enforce boundaries on the start delay Maxime Ripard
@ 2019-02-11 14:41 ` Maxime Ripard
  2019-02-12 15:41   ` Paul Kocialkowski
  2019-02-11 14:41 ` [PATCH v3 5/8] drm/sun4i: dsi: Fix DRQ calculation Maxime Ripard
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-02-11 14:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Sean Paul, Maxime Ripard
  Cc: bbrezillon, dri-devel, Paul Kocialkowski, Chen-Yu Tsai,
	Thomas Petazzoni, Jagan Teki, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 5/8] drm/sun4i: dsi: Fix DRQ calculation
  2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
                   ` (3 preceding siblings ...)
  2019-02-11 14:41 ` [PATCH v3 4/8] drm/sun4i: dsi: Fix front vs back porch calculation Maxime Ripard
@ 2019-02-11 14:41 ` Maxime Ripard
  2019-02-13 14:33   ` Paul Kocialkowski
  2019-02-11 14:41 ` [PATCH v3 6/8] drm/sun4i: dsi: Rework a bit the hblk calculation Maxime Ripard
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-02-11 14:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Sean Paul, Maxime Ripard
  Cc: bbrezillon, dri-devel, Paul Kocialkowski, Chen-Yu Tsai,
	Thomas Petazzoni, Jagan Teki, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 6/8] drm/sun4i: dsi: Rework a bit the hblk calculation
  2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
                   ` (4 preceding siblings ...)
  2019-02-11 14:41 ` [PATCH v3 5/8] drm/sun4i: dsi: Fix DRQ calculation Maxime Ripard
@ 2019-02-11 14:41 ` Maxime Ripard
  2019-02-13 14:41   ` Paul Kocialkowski
  2019-02-11 14:41 ` [PATCH v3 7/8] drm/sun4i: dsi: Add burst support Maxime Ripard
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-02-11 14:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Sean Paul, Maxime Ripard
  Cc: bbrezillon, dri-devel, Paul Kocialkowski, Chen-Yu Tsai,
	Thomas Petazzoni, Jagan Teki, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 7/8] drm/sun4i: dsi: Add burst support
  2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
                   ` (5 preceding siblings ...)
  2019-02-11 14:41 ` [PATCH v3 6/8] drm/sun4i: dsi: Rework a bit the hblk calculation Maxime Ripard
@ 2019-02-11 14:41 ` Maxime Ripard
  2019-02-13 14:36   ` Paul Kocialkowski
  2019-02-11 14:41 ` [PATCH v3 8/8] drm/panel: Add Ronbo RB070D30 panel Maxime Ripard
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-02-11 14:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Sean Paul, Maxime Ripard
  Cc: Konstantin Sudakov, bbrezillon, dri-devel, Paul Kocialkowski,
	Chen-Yu Tsai, Thomas Petazzoni, Jagan Teki, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 8/8] drm/panel: Add Ronbo RB070D30 panel
  2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
                   ` (6 preceding siblings ...)
  2019-02-11 14:41 ` [PATCH v3 7/8] drm/sun4i: dsi: Add burst support Maxime Ripard
@ 2019-02-11 14:41 ` Maxime Ripard
  2019-02-11 15:13   ` Sam Ravnborg
  2019-02-13 14:26   ` Paul Kocialkowski
  2019-02-12 10:32 ` [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Jagan Teki
  2019-02-15 14:12 ` Maxime Ripard
  9 siblings, 2 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-02-11 14:41 UTC (permalink / raw)
  To: Maarten Lankhorst, Sean Paul, Maxime Ripard
  Cc: Konstantin Sudakov, bbrezillon, Sam Ravnborg, dri-devel,
	Paul Kocialkowski, Chen-Yu Tsai, Thomas Petazzoni, Jagan Teki,
	linux-arm-kernel

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.

Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
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..cb4f99252aa0
--- /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(&default_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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 8/8] drm/panel: Add Ronbo RB070D30 panel
  2019-02-11 14:41 ` [PATCH v3 8/8] drm/panel: Add Ronbo RB070D30 panel Maxime Ripard
@ 2019-02-11 15:13   ` Sam Ravnborg
  2019-02-13 14:26   ` Paul Kocialkowski
  1 sibling, 0 replies; 29+ messages in thread
From: Sam Ravnborg @ 2019-02-11 15:13 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Konstantin Sudakov, bbrezillon, Maarten Lankhorst, dri-devel,
	Paul Kocialkowski, Chen-Yu Tsai, Sean Paul, Thomas Petazzoni,
	Jagan Teki, linux-arm-kernel

Hi Maxime.

Looks good, but spotted two small issues.

> +#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>
These are no longer used as far as I can see.


> +
> +	/* Reset */

/* Enable power and de-assert reset */?

> +	msleep(20);
> +	gpiod_set_value(ctx->gpios.power, 1);
> +	msleep(20);
> +	gpiod_set_value(ctx->gpios.reset, 1);
> +	msleep(20);


	Sam


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support
  2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
                   ` (7 preceding siblings ...)
  2019-02-11 14:41 ` [PATCH v3 8/8] drm/panel: Add Ronbo RB070D30 panel Maxime Ripard
@ 2019-02-12 10:32 ` Jagan Teki
  2019-02-15 14:12 ` Maxime Ripard
  9 siblings, 0 replies; 29+ messages in thread
From: Jagan Teki @ 2019-02-12 10:32 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: bbrezillon, Maarten Lankhorst, dri-devel, Paul Kocialkowski,
	Chen-Yu Tsai, Sean Paul, Thomas Petazzoni, Jagan Teki,
	linux-arm-kernel

Hi Maxime,

On Mon, Feb 11, 2019 at 8:12 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> 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,

I have similar burst changes[1] (from 01/23 to 09/23) which are now
v7, since these were generic changes and been in ML since from last 6
months. request you to pick or work on top.

> Maxime
>
> Changes from v3:
>   - Fixed error in Ronbo panel error path
>
> 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 (6):
>   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

Same patches are been for a while in ML[2], hope you can consider to pick.

>   drm/sun4i: dsi: Fix DRQ calculation
>   drm/sun4i: dsi: Rework a bit the hblk calculation
>
>  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 +-
>  6 files changed, 419 insertions(+), 42 deletions(-)
>  create mode 100644 drivers/gpu/drm/panel/panel-ronbo-rb070d30.c

[2] https://patchwork.kernel.org/patch/10793123/
     https://patchwork.kernel.org/patch/10793125/
     https://patchwork.kernel.org/patch/10680309/
[1] https://patchwork.kernel.org/cover/10792981/

Jagan.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 2/8] drm/sun4i: dsi: Change the start delay calculation
  2019-02-11 14:41 ` [PATCH v3 2/8] drm/sun4i: dsi: Change the start delay calculation Maxime Ripard
@ 2019-02-12 15:28   ` Paul Kocialkowski
  2019-02-12 15:45     ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Paul Kocialkowski @ 2019-02-12 15:28 UTC (permalink / raw)
  To: Maxime Ripard, Maarten Lankhorst, Sean Paul
  Cc: bbrezillon, dri-devel, Chen-Yu Tsai, Thomas Petazzoni,
	Jagan Teki, linux-arm-kernel

Hi,

On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard 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.

For future reference, it look like there is a version of the BSP where
this is clamped between 8 and 31 in the legacy ebios code:
https://github.com/Icenowy/linux-kernel-lichee-a33/blob/master/drivers/video/sunxi/legacy/disp/de_bsp/de/ebios/de_dsi.c#L687

As well as in the A23 code:
https://github.com/Icenowy/linux-kernel-lichee-a33/blob/master/drivers/video/sunxi/disp/de/lowlevel_sun8iw3/de_dsi.c#L686

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

Looking at the formula from the BSP, I get a different result from
yours.

From what I can see, the BSP formula is:
start_delay = vtotal - vfp + start
vfp = vtotal - y - vbp

With vbp counting the back porch and sync length.
So the final value can be simplified:
start_delay = vbp + y + start

Translated to DRM:
start_delay = vtotal - vsync_start + vdisplay + start

Which differs from your formula by the sync length.

Does this also work out for your hardware? I guess it's best to stick
close to the BSP if we can.

Cheers,

Paul

>  }
>  
>  static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 3/8] drm/sun4i: dsi: Enforce boundaries on the start delay
  2019-02-11 14:41 ` [PATCH v3 3/8] drm/sun4i: dsi: Enforce boundaries on the start delay Maxime Ripard
@ 2019-02-12 15:30   ` Paul Kocialkowski
  2019-02-14  9:21     ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Paul Kocialkowski @ 2019-02-12 15:30 UTC (permalink / raw)
  To: Maxime Ripard, Maarten Lankhorst, Sean Paul
  Cc: bbrezillon, dri-devel, Chen-Yu Tsai, Thomas Petazzoni,
	Jagan Teki, linux-arm-kernel

Hi,

On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> 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>

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@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;

I wonder whether delay == mode->vtotal would make sense at all. If not,
then we can just apply the modulo unconditionally. But in doubt, let's
keep things this way.

Cheers,

Paul

> +
> +	return max_t(u16, delay, 1);
>  }
>  
>  static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 4/8] drm/sun4i: dsi: Fix front vs back porch calculation
  2019-02-11 14:41 ` [PATCH v3 4/8] drm/sun4i: dsi: Fix front vs back porch calculation Maxime Ripard
@ 2019-02-12 15:41   ` Paul Kocialkowski
  0 siblings, 0 replies; 29+ messages in thread
From: Paul Kocialkowski @ 2019-02-12 15:41 UTC (permalink / raw)
  To: Maxime Ripard, Maarten Lankhorst, Sean Paul
  Cc: bbrezillon, dri-devel, Chen-Yu Tsai, Thomas Petazzoni,
	Jagan Teki, linux-arm-kernel

Hi,

On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> 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>

Food for thoughts: everything indicates that backporch does not count
sync length for the DSI registers (while it does for TCON). So all
these changes look good!

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> ---
>  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) |
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 2/8] drm/sun4i: dsi: Change the start delay calculation
  2019-02-12 15:28   ` Paul Kocialkowski
@ 2019-02-12 15:45     ` Maxime Ripard
  0 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-02-12 15:45 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: bbrezillon, Maarten Lankhorst, dri-devel, Chen-Yu Tsai,
	Sean Paul, Thomas Petazzoni, Jagan Teki, linux-arm-kernel


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

On Tue, Feb 12, 2019 at 04:28:13PM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard 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.
> 
> For future reference, it look like there is a version of the BSP where
> this is clamped between 8 and 31 in the legacy ebios code:
> https://github.com/Icenowy/linux-kernel-lichee-a33/blob/master/drivers/video/sunxi/legacy/disp/de_bsp/de/ebios/de_dsi.c#L687
> 
> As well as in the A23 code:
> https://github.com/Icenowy/linux-kernel-lichee-a33/blob/master/drivers/video/sunxi/disp/de/lowlevel_sun8iw3/de_dsi.c#L686
> 
> > 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;
> 
> Looking at the formula from the BSP, I get a different result from
> yours.
> 
> From what I can see, the BSP formula is:
> start_delay = vtotal - vfp + start
> vfp = vtotal - y - vbp
> 
> With vbp counting the back porch and sync length.

vbp is only the back porch.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 8/8] drm/panel: Add Ronbo RB070D30 panel
  2019-02-11 14:41 ` [PATCH v3 8/8] drm/panel: Add Ronbo RB070D30 panel Maxime Ripard
  2019-02-11 15:13   ` Sam Ravnborg
@ 2019-02-13 14:26   ` Paul Kocialkowski
  2019-02-14 11:07     ` Re[2]: " Konstantin Sudakov
  1 sibling, 1 reply; 29+ messages in thread
From: Paul Kocialkowski @ 2019-02-13 14:26 UTC (permalink / raw)
  To: Maxime Ripard, Maarten Lankhorst, Sean Paul
  Cc: Konstantin Sudakov, bbrezillon, Sam Ravnborg, dri-devel,
	Chen-Yu Tsai, Thomas Petazzoni, Jagan Teki, linux-arm-kernel

Hi,

On Mon, 2019-02-11 at 15:41 +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.
> 
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 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 ++++++++++++++++++++-

Looks like no bindings documentation is introduced for this panel.
Shouldn't that be the case? The vendor also seems to be missing from
vendor-prefixes.txt.

For the driver itself:

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

>  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..cb4f99252aa0
> --- /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(&default_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");
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/8] drm/sun4i: dsi: Fix DRQ calculation
  2019-02-11 14:41 ` [PATCH v3 5/8] drm/sun4i: dsi: Fix DRQ calculation Maxime Ripard
@ 2019-02-13 14:33   ` Paul Kocialkowski
  2019-02-14  9:23     ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Paul Kocialkowski @ 2019-02-13 14:33 UTC (permalink / raw)
  To: Maxime Ripard, Maarten Lankhorst, Sean Paul
  Cc: bbrezillon, dri-devel, Chen-Yu Tsai, Thomas Petazzoni,
	Jagan Teki, linux-arm-kernel

Hi,

On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> 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>

Assuming that the BSP's hbp is referring to the back porch only, then
calculating ht - x - hbp does give us the front porch + hsync length.

Well, let's guess that this about the front porch only until we know
better. It's certainly better than using the back porch anyway :)

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> ---
>  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;
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 7/8] drm/sun4i: dsi: Add burst support
  2019-02-11 14:41 ` [PATCH v3 7/8] drm/sun4i: dsi: Add burst support Maxime Ripard
@ 2019-02-13 14:36   ` Paul Kocialkowski
  2019-02-14  9:15     ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Paul Kocialkowski @ 2019-02-13 14:36 UTC (permalink / raw)
  To: Maxime Ripard, Maarten Lankhorst, Sean Paul
  Cc: Konstantin Sudakov, bbrezillon, dri-devel, Chen-Yu Tsai,
	Thomas Petazzoni, Jagan Teki, linux-arm-kernel

Hi,

On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> 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

[...]

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

It looks a bit strange to zero these variables here while basic_ctl is
initialized to zero when declared. I think it would be more consistent
to have these variables set to zero in the same fashion.

With that fixed:

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> +		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,
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 6/8] drm/sun4i: dsi: Rework a bit the hblk calculation
  2019-02-11 14:41 ` [PATCH v3 6/8] drm/sun4i: dsi: Rework a bit the hblk calculation Maxime Ripard
@ 2019-02-13 14:41   ` Paul Kocialkowski
  0 siblings, 0 replies; 29+ messages in thread
From: Paul Kocialkowski @ 2019-02-13 14:41 UTC (permalink / raw)
  To: Maxime Ripard, Maarten Lankhorst, Sean Paul
  Cc: bbrezillon, dri-devel, Chen-Yu Tsai, Thomas Petazzoni,
	Jagan Teki, linux-arm-kernel

Hi,

On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> 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.

This looks consistent with what the BSP is doing for video mode.

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> 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
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 7/8] drm/sun4i: dsi: Add burst support
  2019-02-13 14:36   ` Paul Kocialkowski
@ 2019-02-14  9:15     ` Maxime Ripard
  0 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-02-14  9:15 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: Konstantin Sudakov, bbrezillon, Maarten Lankhorst, dri-devel,
	Chen-Yu Tsai, Sean Paul, Thomas Petazzoni, Jagan Teki,
	linux-arm-kernel


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

On Wed, Feb 13, 2019 at 03:36:48PM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> > 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
> 
> [...]
> 
> > @@ -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;
> 
> It looks a bit strange to zero these variables here while basic_ctl is
> initialized to zero when declared. I think it would be more consistent
> to have these variables set to zero in the same fashion.

You're right, I've fixed this while applying.

> With that fixed:
> 
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 3/8] drm/sun4i: dsi: Enforce boundaries on the start delay
  2019-02-12 15:30   ` Paul Kocialkowski
@ 2019-02-14  9:21     ` Maxime Ripard
  0 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-02-14  9:21 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: bbrezillon, Maarten Lankhorst, dri-devel, Chen-Yu Tsai,
	Sean Paul, Thomas Petazzoni, Jagan Teki, linux-arm-kernel


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

hi,

On Tue, Feb 12, 2019 at 04:30:33PM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> > 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>
> 
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Thanks!

> > ---
> >  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;
> 
> I wonder whether delay == mode->vtotal would make sense at all. If not,
> then we can just apply the modulo unconditionally. But in doubt, let's
> keep things this way.

I guess that situation can happen, especially with the start clamping.

But even without it, it can happen when mode->vtotal - mode->vsync_end
(so the backporch) is equal to 10.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/8] drm/sun4i: dsi: Fix DRQ calculation
  2019-02-13 14:33   ` Paul Kocialkowski
@ 2019-02-14  9:23     ` Maxime Ripard
  0 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-02-14  9:23 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: bbrezillon, Maarten Lankhorst, dri-devel, Chen-Yu Tsai,
	Sean Paul, Thomas Petazzoni, Jagan Teki, linux-arm-kernel


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

On Wed, Feb 13, 2019 at 03:33:38PM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> > 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>
> 
> Assuming that the BSP's hbp is referring to the back porch only, then
> calculating ht - x - hbp does give us the front porch + hsync length.
> 
> Well, let's guess that this about the front porch only until we know
> better. It's certainly better than using the back porch anyway :)

After discussing it with Paul, we decided to drop that patch since it
would make the backporch definition inconsistent in the driver.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re[2]: [PATCH v3 8/8] drm/panel: Add Ronbo RB070D30 panel
  2019-02-13 14:26   ` Paul Kocialkowski
@ 2019-02-14 11:07     ` Konstantin Sudakov
  0 siblings, 0 replies; 29+ messages in thread
From: Konstantin Sudakov @ 2019-02-14 11:07 UTC (permalink / raw)
  To: Paul Kocialkowski, Maxime Ripard
  Cc: bbrezillon, Sam Ravnborg, Maarten Lankhorst, dri-devel,
	Chen-Yu Tsai, Sean Paul, Thomas Petazzoni, Jagan Teki,
	linux-arm-kernel

Hello Paul, Maxime. I suggest the patch to fix it.

diff --no-dereference -u -r -N linux-5.0-rc6_origin/Documentation/devicetree/bindings/display/panel/ronbo,rb070d30.txt linux-5.0-rc6/Documentation/devicetree/bindings/display/panel/ronbo,rb070d30.txt
--- linux-5.0-rc6_origin/Documentation/devicetree/bindings/display/panel/ronbo,rb070d30.txt	1970-01-01 07:00:00.000000000 +0700
+++ linux-5.0-rc6/Documentation/devicetree/bindings/display/panel/ronbo,rb070d30.txt	2019-02-14 18:00:06.613795857 +0700
@@ -0,0 +1,23 @@
+Ronbo RB070D30N01A MIPI-DSI panel
+
+Required properties:
+	- compatible: must be "ronbo,rb070d30"
+	- reg: DSI virtual channel used by that screen
+	- vcc-lcd-supply: phandle to the power regulator
+	- backlight: phandle to the backlight used
+	- reset-gpios: a GPIO phandle for the reset pin
+	- power-gpios: a GPIO phandle for the power pin
+	- updn-gpios: a GPIO phandle for the updn pin (vertical flip)
+	- shlr-gpios: a GPIO phandle for the shlr pin (horizontal flip)
+
+Example:
+panel@0 {
+	compatible = "ronbo,rb070d30";
+	reg = <0>;
+	vcc-lcd-supply = <&reg_display>;
+	backlight = <&backlight>;
+	reset-gpios = <&r_pio 0 1 GPIO_ACTIVE_HIGH>; /* PL01 */
+	power-gpios = <&r_pio 0 2 GPIO_ACTIVE_HIGH>; /* PL02 */
+	updn-gpios = <&r_pio 0 3 GPIO_ACTIVE_HIGH>; /* PL03 */
+	shlr-gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>; /* PL04 */
+};
diff --no-dereference -u -r -N linux-5.0-rc6_origin/Documentation/devicetree/bindings/vendor-prefixes.txt linux-5.0-rc6/Documentation/devicetree/bindings/vendor-prefixes.txt
--- linux-5.0-rc6_origin/Documentation/devicetree/bindings/vendor-prefixes.txt	2019-02-11 05:42:20.000000000 +0700
+++ linux-5.0-rc6/Documentation/devicetree/bindings/vendor-prefixes.txt	2019-02-14 17:39:46.667994322 +0700
@@ -334,6 +334,7 @@
 riscv	RISC-V Foundation
 rockchip	Fuzhou Rockchip Electronics Co., Ltd
 rohm	ROHM Semiconductor Co., Ltd
+ronbo	Ronbo Electronics Ltd.
 roofull	Shenzhen Roofull Technology Co, Ltd
 samsung	Samsung Semiconductor
 samtec	Samtec/Softing company
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support
  2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
                   ` (8 preceding siblings ...)
  2019-02-12 10:32 ` [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Jagan Teki
@ 2019-02-15 14:12 ` Maxime Ripard
       [not found]   ` <CAMty3ZAA90-fHPADJSE3ht9CiYWA3yBoyEy7wVv9=6C5JiaTVw@mail.gmail.com>
  9 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-02-15 14:12 UTC (permalink / raw)
  To: Maarten Lankhorst, Sean Paul
  Cc: bbrezillon, dri-devel, Paul Kocialkowski, Chen-Yu Tsai,
	Thomas Petazzoni, Jagan Teki, linux-arm-kernel


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

On Mon, Feb 11, 2019 at 03:41:21PM +0100, Maxime Ripard wrote:
> 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.

I should have sent that mail yesterday, but patches 1-4 and 6-7 were
merged. Patch 5 was discarded since it was not consistent with the
rest of the driver, and 8 had some comments.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support
       [not found]   ` <CAMty3ZAA90-fHPADJSE3ht9CiYWA3yBoyEy7wVv9=6C5JiaTVw@mail.gmail.com>
@ 2019-02-15 17:07     ` Jagan Teki
  2019-02-18  8:26       ` Paul Kocialkowski
  0 siblings, 1 reply; 29+ messages in thread
From: Jagan Teki @ 2019-02-15 17:07 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: bbrezillon, Maarten Lankhorst, dri-devel, Paul Kocialkowski,
	Chen-Yu Tsai, Sean Paul, Thomas Petazzoni, linux-arm-kernel

Hi Maxime,

On 15/02/19 8:10 PM, Jagan Teki wrote:
> 
> 
> On Fri, 15 Feb, 2019, 7:43 PM Maxime Ripard, <maxime.ripard@bootlin.com 
> <mailto:maxime.ripard@bootlin.com>> wrote:
> 
>     On Mon, Feb 11, 2019 at 03:41:21PM +0100, Maxime Ripard wrote:
>      > 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.
> 
>     I should have sent that mail yesterday, but patches 1-4 and 6-7 were
>     merged. Patch 5 was discarded since it was not consistent with the
>     rest of the driver, and 8 had some comments.
> 
> 
> Are the applied patches from this series or from my v7 series?
> 
>   Would you please point me the branch.
> 

Unfortunately my last mail didn't reach arm mailing list.

Just wanted to know are the applied patches from this series or from my 
v7 series? Would you please point me the repo, I couldn't find it on
git://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git

Jagan.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support
  2019-02-15 17:07     ` Jagan Teki
@ 2019-02-18  8:26       ` Paul Kocialkowski
  2019-02-18 10:31         ` Jagan Teki
  0 siblings, 1 reply; 29+ messages in thread
From: Paul Kocialkowski @ 2019-02-18  8:26 UTC (permalink / raw)
  To: Jagan Teki, Maxime Ripard
  Cc: bbrezillon, Maarten Lankhorst, dri-devel, Chen-Yu Tsai,
	Sean Paul, Thomas Petazzoni, linux-arm-kernel

Hi Jagan,

On Fri, 2019-02-15 at 22:37 +0530, Jagan Teki wrote:
> Hi Maxime,
> 
> On 15/02/19 8:10 PM, Jagan Teki wrote:
> > 
> > On Fri, 15 Feb, 2019, 7:43 PM Maxime Ripard, <maxime.ripard@bootlin.com 
> > <mailto:maxime.ripard@bootlin.com>> wrote:
> > 
> >     On Mon, Feb 11, 2019 at 03:41:21PM +0100, Maxime Ripard wrote:
> >      > 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.
> > 
> >     I should have sent that mail yesterday, but patches 1-4 and 6-7 were
> >     merged. Patch 5 was discarded since it was not consistent with the
> >     rest of the driver, and 8 had some comments.
> > 
> > 
> > Are the applied patches from this series or from my v7 series?
> > 
> >   Would you please point me the branch.
> > 
> 
> Unfortunately my last mail didn't reach arm mailing list.
> 
> Just wanted to know are the applied patches from this series or from my 
> v7 series? Would you please point me the repo, I couldn't find it on
> git://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git

This series is the one that was applied upstream. You can find the
commits merged at: https://cgit.freedesktop.org/drm/drm-misc/log/

The sunxi tree is not used for changes taking place in specific
subsystems that have their own maintainer trees. In this case, changes
to the DRM driver are going through the DRM misc tree (of which Maxime
is also a maintainer).

With this basis merged, you should be able to respin your series in a
lighter form. Hopefully, that'll help get your changes in shape faster!

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support
  2019-02-18  8:26       ` Paul Kocialkowski
@ 2019-02-18 10:31         ` Jagan Teki
  2019-02-20 16:35           ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Jagan Teki @ 2019-02-18 10:31 UTC (permalink / raw)
  To: Paul Kocialkowski, Maxime Ripard
  Cc: bbrezillon, dri-devel, Chen-Yu Tsai, Sean Paul, Thomas Petazzoni,
	Jagan Teki, linux-arm-kernel

Hi Paul and Maxime,

On Mon, Feb 18, 2019 at 1:56 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi Jagan,
>
> On Fri, 2019-02-15 at 22:37 +0530, Jagan Teki wrote:
> > Hi Maxime,
> >
> > On 15/02/19 8:10 PM, Jagan Teki wrote:
> > >
> > > On Fri, 15 Feb, 2019, 7:43 PM Maxime Ripard, <maxime.ripard@bootlin.com
> > > <mailto:maxime.ripard@bootlin.com>> wrote:
> > >
> > >     On Mon, Feb 11, 2019 at 03:41:21PM +0100, Maxime Ripard wrote:
> > >      > 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.
> > >
> > >     I should have sent that mail yesterday, but patches 1-4 and 6-7 were
> > >     merged. Patch 5 was discarded since it was not consistent with the
> > >     rest of the driver, and 8 had some comments.
> > >
> > >
> > > Are the applied patches from this series or from my v7 series?
> > >
> > >   Would you please point me the branch.
> > >
> >
> > Unfortunately my last mail didn't reach arm mailing list.
> >
> > Just wanted to know are the applied patches from this series or from my
> > v7 series? Would you please point me the repo, I couldn't find it on
> > git://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git
>
> This series is the one that was applied upstream. You can find the
> commits merged at: https://cgit.freedesktop.org/drm/drm-misc/log/

Thanks for sharing the link Paul.

This is really really discouraging.

Don't know whom to ask directly about this, but I am really upset
about this move.

Most of the changes from applied series have similar patches that are
been part of my series of patches. I've been sending this since last
September (which was sent way prior than this series).

How come the same series is recreated and applied with minor changes
while the original series was still in discussion. At least Maxime
should have informed me or he should have rejected my work from
patchwork or atleast NAK in ML?

All these burst changes and random fixes are reviewed in couple of
versions, now the versioning moved to v8[1] [2]. For each and every
versioning I'm trying to fix the previous version comments, code
improvements, commit messages. In fact for each rotation I'm trying to
validate 4 different panels which eventually consume all my 16 hours
in day.

Please let me know to how could we better collaborate?

[2] https://patchwork.kernel.org/cover/10813573/
[1] https://patchwork.kernel.org/cover/10813623/

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support
  2019-02-18 10:31         ` Jagan Teki
@ 2019-02-20 16:35           ` Maxime Ripard
  2019-02-26  6:48             ` Jagan Teki
  0 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-02-20 16:35 UTC (permalink / raw)
  To: Jagan Teki
  Cc: bbrezillon, dri-devel, Paul Kocialkowski, Chen-Yu Tsai,
	Sean Paul, Thomas Petazzoni, Jagan Teki, linux-arm-kernel

Hi,

On Mon, Feb 18, 2019 at 04:01:09PM +0530, Jagan Teki wrote:
> On Mon, Feb 18, 2019 at 1:56 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > On Fri, 2019-02-15 at 22:37 +0530, Jagan Teki wrote:
> > > On 15/02/19 8:10 PM, Jagan Teki wrote:
> > > >
> > > > On Fri, 15 Feb, 2019, 7:43 PM Maxime Ripard, <maxime.ripard@bootlin.com
> > > > <mailto:maxime.ripard@bootlin.com>> wrote:
> > > >
> > > >     On Mon, Feb 11, 2019 at 03:41:21PM +0100, Maxime Ripard wrote:
> > > >      > 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.
> > > >
> > > >     I should have sent that mail yesterday, but patches 1-4 and 6-7 were
> > > >     merged. Patch 5 was discarded since it was not consistent with the
> > > >     rest of the driver, and 8 had some comments.
> > > >
> > > >
> > > > Are the applied patches from this series or from my v7 series?
> > > >
> > > >   Would you please point me the branch.
> > > >
> > >
> > > Unfortunately my last mail didn't reach arm mailing list.
> > >
> > > Just wanted to know are the applied patches from this series or from my
> > > v7 series? Would you please point me the repo, I couldn't find it on
> > > git://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git
> >
> > This series is the one that was applied upstream. You can find the
> > commits merged at: https://cgit.freedesktop.org/drm/drm-misc/log/
> 
> Thanks for sharing the link Paul.
> 
> This is really really discouraging.
> 
> Don't know whom to ask directly about this, but I am really upset
> about this move.

I appreciate and understand that, and I feel sorry it ended up like
this.

> Most of the changes from applied series have similar patches that are
> been part of my series of patches. I've been sending this since last
> September (which was sent way prior than this series).

Note that only the burst part has been merged, and the first time you
sent it was in November.

> How come the same series is recreated and applied with minor changes
> while the original series was still in discussion. At least Maxime
> should have informed me or he should have rejected my work from
> patchwork or atleast NAK in ML?

I did, both in private and public. And I've told you on numerous
occasions what was wrong with your series and the way you were pushing
things. But let's break it down:

v8:
  - Chen-Yu and I spent a lot of time (almost two full work days in my
    case, Chen-Yu at least a full evening from what I know) trying to
    make sense of the Allwinner BSP code, and report what was being
    done. This was made public on the mailing lists, and you were in
    cc [1][2]. It happened the week before your submission, yet you
    ignored most of those changes, and I told you so [3], mentionning
    a bunch of other recurring comments I had that were not really
    addressed.
  - This series and the other also had some obvious flaw that had 0
    chance to work properly (which you eventually noticed[4])

v7:
  - Chen-Yu and I were already discussing and pointing out some
    issues, that were not addressed[5]

v6:
  - Reviewing a PLL issue, already mentionned in the v5 and v2 [6]

v5:
  - I mention that the display I have is broken, just like in your v4 [6].
    Just like in the v4, I'm asking for a panel datasheet so
    that I can help you debug this further. This is ignored.
  - I asked for clarifications on that PLL min_rate, just like in the
    previous versions [7]

v4 
 - I mention that the only other DSI display there is is broken
   [8]. I'm again asking for datasheet and better commit logs.

v3
  - Some more ignored comments [9]

A64 DSI v2
  - Asking details on the PLL min_rate, comments ignored [10]
  - Untested code [11]

Burst v2
  - More details asked, obvious flaws [12]

v1
  - Me asking for better commit logs and some justifications [13][14][15]


TL; DR: there's not been any single iteration of those patches where
you wouldn't have ignored some comments made on a previous iteration,
despite for some patches numerous questions around the same points,
and with very significant time invested in this by numerous people
(Chen-Yu, myself and Paul to a lesser extent).

This is what was completely stalling your series, and I'm sure
frustrating both sides. You even acknowledged that in that mail:
http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/632060.html

Yet, you submitted your v8 versions without taking our comments into
account.

> All these burst changes and random fixes are reviewed in couple of
> versions, now the versioning moved to v8[1] [2]. For each and every
> versioning I'm trying to fix the previous version comments, code
> improvements, commit messages. In fact for each rotation I'm trying to
> validate 4 different panels which eventually consume all my 16 hours
> in day.
> 
> Please let me know to how could we better collaborate?

By listening and addressing the reviews we are making. If you feel
like you're missing something and / or not understanding everything in
a review (which honestly would be pretty understandable with that
sub-par DSI block documentation), then please ask, but ignoring those
comments will just be a waste of time for everyone involved, and will
frustrate everybody (especially when the time spent is this important).

Like I was saying before, I haven't merged any A64 or panel patches,
so this will be a pretty good occasion to test this.

Maxime

1: http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/630522.html
2: http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/630772.html
3: http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/632076.html
4: https://patchwork.freedesktop.org/patch/286191/
5: https://patchwork.freedesktop.org/patch/280045/
6: https://patchwork.freedesktop.org/patch/253495/
7: https://patchwork.freedesktop.org/patch/267373/
8: https://patchwork.freedesktop.org/patch/267362/
9: https://patchwork.freedesktop.org/patch/261779/
10: https://patchwork.freedesktop.org/patch/258921/
11: https://patchwork.kernel.org/patch/10653309/
12: https://patchwork.kernel.org/patch/10653355/
13: https://patchwork.freedesktop.org/patch/262554/
14: https://patchwork.freedesktop.org/patch/253495/
15: https://patchwork.freedesktop.org/patch/253503/
16: https://patchwork.freedesktop.org/patch/253491/

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

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support
  2019-02-20 16:35           ` Maxime Ripard
@ 2019-02-26  6:48             ` Jagan Teki
  0 siblings, 0 replies; 29+ messages in thread
From: Jagan Teki @ 2019-02-26  6:48 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: bbrezillon, dri-devel, Paul Kocialkowski, Chen-Yu Tsai,
	Sean Paul, Thomas Petazzoni, Jagan Teki, linux-arm-kernel

On Wed, Feb 20, 2019 at 10:05 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> On Mon, Feb 18, 2019 at 04:01:09PM +0530, Jagan Teki wrote:
> > On Mon, Feb 18, 2019 at 1:56 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > On Fri, 2019-02-15 at 22:37 +0530, Jagan Teki wrote:
> > > > On 15/02/19 8:10 PM, Jagan Teki wrote:
> > > > >
> > > > > On Fri, 15 Feb, 2019, 7:43 PM Maxime Ripard, <maxime.ripard@bootlin.com
> > > > > <mailto:maxime.ripard@bootlin.com>> wrote:
> > > > >
> > > > >     On Mon, Feb 11, 2019 at 03:41:21PM +0100, Maxime Ripard wrote:
> > > > >      > 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.
> > > > >
> > > > >     I should have sent that mail yesterday, but patches 1-4 and 6-7 were
> > > > >     merged. Patch 5 was discarded since it was not consistent with the
> > > > >     rest of the driver, and 8 had some comments.
> > > > >
> > > > >
> > > > > Are the applied patches from this series or from my v7 series?
> > > > >
> > > > >   Would you please point me the branch.
> > > > >
> > > >
> > > > Unfortunately my last mail didn't reach arm mailing list.
> > > >
> > > > Just wanted to know are the applied patches from this series or from my
> > > > v7 series? Would you please point me the repo, I couldn't find it on
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git
> > >
> > > This series is the one that was applied upstream. You can find the
> > > commits merged at: https://cgit.freedesktop.org/drm/drm-misc/log/
> >
> > Thanks for sharing the link Paul.
> >
> > This is really really discouraging.
> >
> > Don't know whom to ask directly about this, but I am really upset
> > about this move.
>
> I appreciate and understand that, and I feel sorry it ended up like
> this.
>
> > Most of the changes from applied series have similar patches that are
> > been part of my series of patches. I've been sending this since last
> > September (which was sent way prior than this series).
>
> Note that only the burst part has been merged, and the first time you
> sent it was in November.

at least those were prior to the applied series.

>
> > How come the same series is recreated and applied with minor changes
> > while the original series was still in discussion. At least Maxime
> > should have informed me or he should have rejected my work from
> > patchwork or atleast NAK in ML?
>
> I did, both in private and public. And I've told you on numerous
> occasions what was wrong with your series and the way you were pushing
> things. But let's break it down:
>
> v8:
>   - Chen-Yu and I spent a lot of time (almost two full work days in my
>     case, Chen-Yu at least a full evening from what I know) trying to
>     make sense of the Allwinner BSP code, and report what was being
>     done. This was made public on the mailing lists, and you were in
>     cc [1][2]. It happened the week before your submission, yet you

I really missed this, since I don't have any information from you
about sending my burst changes on behalf of other series. This is
something that I couldn't aware in community project where someone
sent the existing changes w/o informing the real author.

>     ignored most of those changes, and I told you so [3], mentionning
>     a bunch of other recurring comments I had that were not really
>     addressed.

On the other hand, I replied about the real reason why I sent this on
[3.1], these changes were generic and doesn't related to clock. You
have been mentioned about the clock but ie not the reason for holding
the generic changes I suppose.

At one point, I felt myself I'm making confusion with big changes, so
I realized and sent the v7 [1] [2] which would eventually break the
changes into sets those are placed generic burst and A64 support. I
don't know I couldn't see any comments.

>   - This series and the other also had some obvious flaw that had 0
>     chance to work properly (which you eventually noticed[4])
>
> v7:
>   - Chen-Yu and I were already discussing and pointing out some
>     issues, that were not addressed[5]
>
> v6:
>   - Reviewing a PLL issue, already mentionned in the v5 and v2 [6]
>
> v5:
>   - I mention that the display I have is broken, just like in your v4 [6].
>     Just like in the v4, I'm asking for a panel datasheet so
>     that I can help you debug this further. This is ignored.
>   - I asked for clarifications on that PLL min_rate, just like in the
>     previous versions [7]
>
> v4
>  - I mention that the only other DSI display there is is broken
>    [8]. I'm again asking for datasheet and better commit logs.
>
> v3
>   - Some more ignored comments [9]
>
> A64 DSI v2
>   - Asking details on the PLL min_rate, comments ignored [10]
>   - Untested code [11]
>
> Burst v2
>   - More details asked, obvious flaws [12]
>
> v1
>   - Me asking for better commit logs and some justifications [13][14][15]
>
>
> TL; DR: there's not been any single iteration of those patches where
> you wouldn't have ignored some comments made on a previous iteration,
> despite for some patches numerous questions around the same points,
> and with very significant time invested in this by numerous people
> (Chen-Yu, myself and Paul to a lesser extent).
>
> This is what was completely stalling your series, and I'm sure
> frustrating both sides. You even acknowledged that in that mail:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/632060.html
>
> Yet, you submitted your v8 versions without taking our comments into
> account.
>
> > All these burst changes and random fixes are reviewed in couple of
> > versions, now the versioning moved to v8[1] [2]. For each and every
> > versioning I'm trying to fix the previous version comments, code
> > improvements, commit messages. In fact for each rotation I'm trying to
> > validate 4 different panels which eventually consume all my 16 hours
> > in day.
> >
> > Please let me know to how could we better collaborate?
>
> By listening and addressing the reviews we are making. If you feel
> like you're missing something and / or not understanding everything in
> a review (which honestly would be pretty understandable with that
> sub-par DSI block documentation), then please ask, but ignoring those
> comments will just be a waste of time for everyone involved, and will
> frustrate everybody (especially when the time spent is this important).

I did address all your comments as per as generic burst changes, which
were supposed to fixed in initial versions. here are the changelog for
your information.

Changes for v8:
- rebase on master
- rework on commit messages
- include drq changes from previous version
Changes for v7:
- rebase on master
- collect Merlijn Wajer Tested-by credits.
Changes for v6:
- fixed all burst mode patches as per previous version comments
- rebase on master
- update proper commit message
- dropped unneeded comments
- order the patches that make review easy
Changes for v5, v4, v3, v2:
- use proper return value for tcon0 probe
- add logic to get tcon0 divider values
- simplify timings code to support burst mode
- fix drq computation return values
- update proper commit messages
- rebase on master

>
> Like I was saying before, I haven't merged any A64 or panel patches,
> so this will be a pretty good occasion to test this.

Honestly I didn't frustrate about this work, and  I couldn't see any
proper reason for your response on why you send similar burst changes
w/o informing or NAK my changes. I assume you may feel confused or
frustrated about my series (which I didn't intentionally do that
sorry), so you mark the changes from your side and applied.

Anyway, thanks for your support, I will re-spin my changes on top
these applied changes.

[3.1] http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/632060.html
[2] https://patchwork.kernel.org/cover/10813623//
[1] https://patchwork.kernel.org/cover/10792981/

Jagan.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2019-02-26  6:49 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11 14:41 [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Maxime Ripard
2019-02-11 14:41 ` [PATCH v3 1/8] drm/sun4i: dsi: Restrict DSI tcon clock divider Maxime Ripard
2019-02-11 14:41 ` [PATCH v3 2/8] drm/sun4i: dsi: Change the start delay calculation Maxime Ripard
2019-02-12 15:28   ` Paul Kocialkowski
2019-02-12 15:45     ` Maxime Ripard
2019-02-11 14:41 ` [PATCH v3 3/8] drm/sun4i: dsi: Enforce boundaries on the start delay Maxime Ripard
2019-02-12 15:30   ` Paul Kocialkowski
2019-02-14  9:21     ` Maxime Ripard
2019-02-11 14:41 ` [PATCH v3 4/8] drm/sun4i: dsi: Fix front vs back porch calculation Maxime Ripard
2019-02-12 15:41   ` Paul Kocialkowski
2019-02-11 14:41 ` [PATCH v3 5/8] drm/sun4i: dsi: Fix DRQ calculation Maxime Ripard
2019-02-13 14:33   ` Paul Kocialkowski
2019-02-14  9:23     ` Maxime Ripard
2019-02-11 14:41 ` [PATCH v3 6/8] drm/sun4i: dsi: Rework a bit the hblk calculation Maxime Ripard
2019-02-13 14:41   ` Paul Kocialkowski
2019-02-11 14:41 ` [PATCH v3 7/8] drm/sun4i: dsi: Add burst support Maxime Ripard
2019-02-13 14:36   ` Paul Kocialkowski
2019-02-14  9:15     ` Maxime Ripard
2019-02-11 14:41 ` [PATCH v3 8/8] drm/panel: Add Ronbo RB070D30 panel Maxime Ripard
2019-02-11 15:13   ` Sam Ravnborg
2019-02-13 14:26   ` Paul Kocialkowski
2019-02-14 11:07     ` Re[2]: " Konstantin Sudakov
2019-02-12 10:32 ` [PATCH v3 0/8] drm/sun4i: dsi: Add burst mode support Jagan Teki
2019-02-15 14:12 ` Maxime Ripard
     [not found]   ` <CAMty3ZAA90-fHPADJSE3ht9CiYWA3yBoyEy7wVv9=6C5JiaTVw@mail.gmail.com>
2019-02-15 17:07     ` Jagan Teki
2019-02-18  8:26       ` Paul Kocialkowski
2019-02-18 10:31         ` Jagan Teki
2019-02-20 16:35           ` Maxime Ripard
2019-02-26  6:48             ` Jagan Teki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).