Re: [PATCH 7/8] drm/sun4i: Add various bits and pieces to enable LVDS support on sun4i

[Maxime Ripard]

On Sat, Feb 11, 2017 at 07:44:04PM +0200, Priit Laes wrote:
> TODO: We still rely on u-boot for lvds reset bit setup :(

That needs to be figured out before merging :/

You also have a number of checkpatch warnings / errors that needs to
be fixed.

> 
> Signed-off-by: Priit Laes 
> ---
>  drivers/gpu/drm/sun4i/sun4i_lvds.c | 29 
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 54 
> --
>  drivers/gpu/drm/sun4i/sun4i_tcon.h | 15 +++
>  3 files changed, 90 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c 
> b/drivers/gpu/drm/sun4i/sun4i_lvds.c
> index 2ba4705..de738e5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_lvds.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c
> @@ -114,6 +114,35 @@ static void sun4i_lvds_encoder_enable(struct drm_encoder 
> *encoder)
>   /* encoder->bridge can be NULL; drm_bridge_enable checks for it */
>   drm_bridge_enable(encoder->bridge);
>  
> + /* Enable the LVDS */
> + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
> +SUN4I_TCON0_LVDS_IF_ENABLE,
> +SUN4I_TCON0_LVDS_IF_ENABLE);
> +
> + /*
> +  * TODO: SUN4I_TCON0_LVDS_ANA0_REG_C and SUN4I_TCON0_LVDS_ANA0_PD
> +  * registers span 3 bits, but we only set upper 2 for both
> +  * of them based on values taken from Allwinner driver.
> +  */
> + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +  SUN4I_TCON0_LVDS_ANA0_CK_EN |
> +  SUN4I_TCON0_LVDS_ANA0_REG_V |
> +  SUN4I_TCON0_LVDS_ANA0_REG_C |
> +  SUN4I_TCON0_LVDS_ANA0_EN_MB |
> +  SUN4I_TCON0_LVDS_ANA0_PD |
> +  SUN4I_TCON0_LVDS_ANA0_DCHS);
> +
> + udelay(2000);
> +
> + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
> +  SUN4I_TCON0_LVDS_ANA1_INIT);
> +
> + udelay(1000);
> +
> + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
> +SUN4I_TCON0_LVDS_ANA1_UPDATE,
> +SUN4I_TCON0_LVDS_ANA1_UPDATE);
> +
>   sun4i_tcon_channel_enable(tcon, 0);

This should be merged in your patch 6.

>  }
>  
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 71d0087..468a3ce 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -18,6 +18,7 @@
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -29,6 +30,7 @@
>  #include "sun4i_crtc.h"
>  #include "sun4i_dotclock.h"
>  #include "sun4i_drv.h"
> +#include "sun4i_lvds.h"
>  #include "sun4i_rgb.h"
>  #include "sun4i_tcon.h"
>  
> @@ -169,12 +171,29 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon,
>SUN4I_TCON0_BASIC2_V_BACKPORCH(bp));
>  
>   /* Set Hsync and Vsync length */
> - hsync = mode->crtc_hsync_end - mode->crtc_hsync_start;
> - vsync = mode->crtc_vsync_end - mode->crtc_vsync_start;
> - DRM_DEBUG_DRIVER("Setting HSYNC %d, VSYNC %d\n", hsync, vsync);
> - regmap_write(tcon->regs, SUN4I_TCON0_BASIC3_REG,
> -  SUN4I_TCON0_BASIC3_V_SYNC(vsync) |
> -  SUN4I_TCON0_BASIC3_H_SYNC(hsync));
> + if (type != DRM_MODE_ENCODER_LVDS) {
> + // Not needed for LVDS?
> + hsync = mode->crtc_hsync_end - mode->crtc_hsync_start;
> + vsync = mode->crtc_vsync_end - mode->crtc_vsync_start;
> + DRM_DEBUG_DRIVER("Setting HSYNC %d, VSYNC %d\n", hsync, vsync);
> + regmap_write(tcon->regs, SUN4I_TCON0_BASIC3_REG,
> +  SUN4I_TCON0_BASIC3_V_SYNC(vsync) |
> +  SUN4I_TCON0_BASIC3_H_SYNC(hsync));
> + }

This is your patch 5 (and it would be better to put the condition on
what we know rather than what we assume, we know that it's working for
RGB, but not for anything else).

> +
> + if (type == DRM_MODE_ENCODER_LVDS) {
> + /* Setup bit depth */
> + /* TODO: Figure out where to get display bit depth
> +  * val = (1: 18-bit, 0: 24-bit)
> +  * TODO: Should we set more registers:
> +  * BIT(28) - LVDS_DIRECTION
> +  * BIT(27) - LVDS_MODE
> +  * BIT(23) - LVDS_CORRECT_MODE
> +  */
> + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
> +SUN4I_TCON0_LVDS_IF_BITWIDTH,
> +SUN4I_TCON0_LVDS_IF_BITWIDTH);
> + }

And this in your patch 6

>  
>   /* Setup the polarity of the various signals */
>   if (!(mode->flags & DRM_MODE_FLAG_PHSYNC))
> @@ -183,8 +202,15 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon,
>   if (!(mode->flags & DRM_MODE_FLAG_PVSYNC))
>   val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
>  
> +
> + /* Set proper DCLK phase value */
> + if (type == DRM_MODE_ENCODER_LVDS)

[PATCH 7/8] drm/sun4i: Add various bits and pieces to enable LVDS support on sun4i

[Priit Laes]

TODO: We still rely on u-boot for lvds reset bit setup :(

Signed-off-by: Priit Laes 
---
 drivers/gpu/drm/sun4i/sun4i_lvds.c | 29 
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 54 --
 drivers/gpu/drm/sun4i/sun4i_tcon.h | 15 +++
 3 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c 
b/drivers/gpu/drm/sun4i/sun4i_lvds.c
index 2ba4705..de738e5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_lvds.c
+++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c
@@ -114,6 +114,35 @@ static void sun4i_lvds_encoder_enable(struct drm_encoder 
*encoder)
/* encoder->bridge can be NULL; drm_bridge_enable checks for it */
drm_bridge_enable(encoder->bridge);
 
+   /* Enable the LVDS */
+   regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
+  SUN4I_TCON0_LVDS_IF_ENABLE,
+  SUN4I_TCON0_LVDS_IF_ENABLE);
+
+   /*
+* TODO: SUN4I_TCON0_LVDS_ANA0_REG_C and SUN4I_TCON0_LVDS_ANA0_PD
+* registers span 3 bits, but we only set upper 2 for both
+* of them based on values taken from Allwinner driver.
+*/
+   regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+SUN4I_TCON0_LVDS_ANA0_CK_EN |
+SUN4I_TCON0_LVDS_ANA0_REG_V |
+SUN4I_TCON0_LVDS_ANA0_REG_C |
+SUN4I_TCON0_LVDS_ANA0_EN_MB |
+SUN4I_TCON0_LVDS_ANA0_PD |
+SUN4I_TCON0_LVDS_ANA0_DCHS);
+
+   udelay(2000);
+
+   regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
+SUN4I_TCON0_LVDS_ANA1_INIT);
+
+   udelay(1000);
+
+   regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
+  SUN4I_TCON0_LVDS_ANA1_UPDATE,
+  SUN4I_TCON0_LVDS_ANA1_UPDATE);
+
sun4i_tcon_channel_enable(tcon, 0);
 }
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 71d0087..468a3ce 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -18,6 +18,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -29,6 +30,7 @@
 #include "sun4i_crtc.h"
 #include "sun4i_dotclock.h"
 #include "sun4i_drv.h"
+#include "sun4i_lvds.h"
 #include "sun4i_rgb.h"
 #include "sun4i_tcon.h"
 
@@ -169,12 +171,29 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon,
 SUN4I_TCON0_BASIC2_V_BACKPORCH(bp));
 
/* Set Hsync and Vsync length */
-   hsync = mode->crtc_hsync_end - mode->crtc_hsync_start;
-   vsync = mode->crtc_vsync_end - mode->crtc_vsync_start;
-   DRM_DEBUG_DRIVER("Setting HSYNC %d, VSYNC %d\n", hsync, vsync);
-   regmap_write(tcon->regs, SUN4I_TCON0_BASIC3_REG,
-SUN4I_TCON0_BASIC3_V_SYNC(vsync) |
-SUN4I_TCON0_BASIC3_H_SYNC(hsync));
+   if (type != DRM_MODE_ENCODER_LVDS) {
+   // Not needed for LVDS?
+   hsync = mode->crtc_hsync_end - mode->crtc_hsync_start;
+   vsync = mode->crtc_vsync_end - mode->crtc_vsync_start;
+   DRM_DEBUG_DRIVER("Setting HSYNC %d, VSYNC %d\n", hsync, vsync);
+   regmap_write(tcon->regs, SUN4I_TCON0_BASIC3_REG,
+SUN4I_TCON0_BASIC3_V_SYNC(vsync) |
+SUN4I_TCON0_BASIC3_H_SYNC(hsync));
+   }
+
+   if (type == DRM_MODE_ENCODER_LVDS) {
+   /* Setup bit depth */
+   /* TODO: Figure out where to get display bit depth
+* val = (1: 18-bit, 0: 24-bit)
+* TODO: Should we set more registers:
+* BIT(28) - LVDS_DIRECTION
+* BIT(27) - LVDS_MODE
+* BIT(23) - LVDS_CORRECT_MODE
+*/
+   regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
+  SUN4I_TCON0_LVDS_IF_BITWIDTH,
+  SUN4I_TCON0_LVDS_IF_BITWIDTH);
+   }
 
/* Setup the polarity of the various signals */
if (!(mode->flags & DRM_MODE_FLAG_PHSYNC))
@@ -183,8 +202,15 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon,
if (!(mode->flags & DRM_MODE_FLAG_PVSYNC))
val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
 
+
+   /* Set proper DCLK phase value */
+   if (type == DRM_MODE_ENCODER_LVDS)
+   val |= SUN4I_TCON0_IO_POL_DCLK_PHASE(1);
+
regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
-  SUN4I_TCON0_IO_POL_HSYNC_POSITIVE | 
SUN4I_TCON0_IO_POL_VSYNC_POSITIVE,
+  SUN4I_TCON0_IO_POL_HSYNC_POSITIVE |
+  SUN4I_TCON0_IO_POL_VSYNC_POSITIVE |
+  SUN4I_TCON0_IO_POL_DCLK_PHASE_MASK,
   val);
 
/* Map output pins to channel 0 */
@@ -480,6 +506,7 @@ static int sun4i_tcon_bind(struct