[v3,7/8] drm/sun4i: dsi: Add burst support

Commit Message

Maxime Ripard Feb. 11, 2019, 2:41 p.m. UTC

From: Konstantin Sudakov <[e-mail]>

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 <[e-mail]>
Signed-off-by: Maxime Ripard <[e-mail]>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 171 ++++++++++++++++++++------
 1 file changed, 132 insertions(+), 39 deletions(-)

Comments

Paul Kocialkowski Feb. 13, 2019, 2:36 p.m. UTC | #1

Hi,

On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> From: Konstantin Sudakov <[e-mail]>
> 
> 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 <[e-mail]>
> Signed-off-by: Maxime Ripard <[e-mail]>
> ---
>  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 <[e-mail]>

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,

Maxime Ripard Feb. 14, 2019, 9:15 a.m. UTC | #2

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 <[e-mail]>
> > 
> > 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 <[e-mail]>
> > Signed-off-by: Maxime Ripard <[e-mail]>
> > ---
> >  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 <[e-mail]>

Thanks!
Maxime

Patch

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,