Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/1] Support LVDS output on Allwinner A20
@ 2020-02-10 19:56 Andrey Lebedev
  2020-02-11  7:20 ` Maxime Ripard
                   ` (6 more replies)
  0 siblings, 7 replies; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-10 19:56 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel

A20 SoC (found in Cubieboard 2 among others) requires different LVDS set
up procedure than A33. Timing controller (tcon) driver only implements
sun6i-style procedure, that doesn't work on A20 (sun7i).

The support for such procedure is ported from u-boot and follows u-boot
naming convention: SUN6I* for sun6i-style procedure, and SUN4I for other
(which happens to be compatible with A20).
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 91 ++++++++++++++++++++----------
 drivers/gpu/drm/sun4i/sun4i_tcon.h | 12 ++++
 2 files changed, 73 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index c81cdce6ed55..78896e907ca9 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -114,46 +114,74 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
 	}
 }
 
+static void sun4i_tcon_lvds_sun6i_enable(struct sun4i_tcon *tcon,
+					 const struct drm_encoder *encoder) {
+	u8 val;
+	regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+		     SUN6I_TCON0_LVDS_ANA0_C(2) |
+		     SUN6I_TCON0_LVDS_ANA0_V(3) |
+		     SUN6I_TCON0_LVDS_ANA0_PD(2) |
+		     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
+	udelay(2);
+
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN6I_TCON0_LVDS_ANA0_EN_MB,
+			   SUN6I_TCON0_LVDS_ANA0_EN_MB);
+	udelay(2);
+
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
+			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
+
+	if (sun4i_tcon_get_pixel_depth(encoder) == 18)
+		val = 7;
+	else
+		val = 0xf;
+
+	regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
+			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
+
+}
+
+static void sun4i_tcon_lvds_sun4i_enable(struct sun4i_tcon *tcon) {
+	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(2); /* delay at least 1200 ns */
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
+			   SUN4I_TCON0_LVDS_ANA1_INIT,
+			   SUN4I_TCON0_LVDS_ANA1_INIT);
+	udelay(1); /* delay at least 1200 ns */
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
+			   SUN4I_TCON0_LVDS_ANA1_UPDATE,
+			   SUN4I_TCON0_LVDS_ANA1_UPDATE);
+}
+
+
 static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon,
 				       const struct drm_encoder *encoder,
 				       bool enabled)
 {
 	if (enabled) {
-		u8 val;
-
+		// Enable LVDS interface
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
 				   SUN4I_TCON0_LVDS_IF_EN,
 				   SUN4I_TCON0_LVDS_IF_EN);
 
-		/*
-		 * As their name suggest, these values only apply to the A31
-		 * and later SoCs. We'll have to rework this when merging
-		 * support for the older SoCs.
-		 */
-		regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-			     SUN6I_TCON0_LVDS_ANA0_C(2) |
-			     SUN6I_TCON0_LVDS_ANA0_V(3) |
-			     SUN6I_TCON0_LVDS_ANA0_PD(2) |
-			     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
-		udelay(2);
-
-		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				   SUN6I_TCON0_LVDS_ANA0_EN_MB,
-				   SUN6I_TCON0_LVDS_ANA0_EN_MB);
-		udelay(2);
-
-		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
-				   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
-
-		if (sun4i_tcon_get_pixel_depth(encoder) == 18)
-			val = 7;
-		else
-			val = 0xf;
+		// Perform SoC-specific setup procedure
+		if (tcon->quirks->sun6i_lvds_init) {
+			sun4i_tcon_lvds_sun6i_enable(tcon, encoder);
+		}
+		else {
+			sun4i_tcon_lvds_sun4i_enable(tcon);
+		}
 
-		regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
-				  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
 	} else {
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
 				   SUN4I_TCON0_LVDS_IF_EN, 0);
@@ -1454,6 +1482,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
 };
 
 static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
+	.supports_lvds		= true,
 	.has_channel_0		= true,
 	.has_channel_1		= true,
 	.dclk_min_div		= 4,
@@ -1464,11 +1493,13 @@ static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
 static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
 	.has_channel_0		= true,
 	.has_lvds_alt		= true,
+	.sun6i_lvds_init	= true,
 	.dclk_min_div		= 1,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = {
 	.supports_lvds		= true,
+	.sun6i_lvds_init	= true,
 	.has_channel_0		= true,
 	.dclk_min_div		= 1,
 };
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index a62ec826ae71..973901c1bee5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -193,6 +193,13 @@
 #define SUN4I_TCON_MUX_CTRL_REG			0x200
 
 #define SUN4I_TCON0_LVDS_ANA0_REG		0x220
+#define SUN4I_TCON0_LVDS_ANA0_DCHS			BIT(16)
+#define SUN4I_TCON0_LVDS_ANA0_PD			BIT(20) | BIT(21)
+#define SUN4I_TCON0_LVDS_ANA0_EN_MB			BIT(22)
+#define SUN4I_TCON0_LVDS_ANA0_REG_C			BIT(24) | BIT(25)
+#define SUN4I_TCON0_LVDS_ANA0_REG_V			BIT(26) | BIT(27)
+#define SUN4I_TCON0_LVDS_ANA0_CK_EN			BIT(29) | BIT(28)
+
 #define SUN6I_TCON0_LVDS_ANA0_EN_MB			BIT(31)
 #define SUN6I_TCON0_LVDS_ANA0_EN_LDO			BIT(30)
 #define SUN6I_TCON0_LVDS_ANA0_EN_DRVC			BIT(24)
@@ -201,6 +208,10 @@
 #define SUN6I_TCON0_LVDS_ANA0_V(x)			(((x) & 3) << 8)
 #define SUN6I_TCON0_LVDS_ANA0_PD(x)			(((x) & 3) << 4)
 
+#define SUN4I_TCON0_LVDS_ANA1_REG		0x224
+#define SUN4I_TCON0_LVDS_ANA1_INIT			(0x1f << 26 | 0x1f << 10)
+#define SUN4I_TCON0_LVDS_ANA1_UPDATE			(0x1f << 16 | 0x1f << 00)
+
 #define SUN4I_TCON1_FILL_CTL_REG		0x300
 #define SUN4I_TCON1_FILL_BEG0_REG		0x304
 #define SUN4I_TCON1_FILL_END0_REG		0x308
@@ -224,6 +235,7 @@ struct sun4i_tcon_quirks {
 	bool	needs_de_be_mux; /* sun6i needs mux to select backend */
 	bool    needs_edp_reset; /* a80 edp reset needed for tcon0 access */
 	bool	supports_lvds;   /* Does the TCON support an LVDS output? */
+	bool	sun6i_lvds_init; /* Requires sun6i lvds initialization? */
 	u8	dclk_min_div;	/* minimum divider for TCON0 DCLK */
 
 	/* callback to handle tcon muxing options */
-- 
2.20.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] 36+ messages in thread

* Re: [PATCH 1/1] Support LVDS output on Allwinner A20
  2020-02-10 19:56 [PATCH 1/1] Support LVDS output on Allwinner A20 Andrey Lebedev
@ 2020-02-11  7:20 ` Maxime Ripard
  2020-02-11 20:48   ` Andrey Lebedev
  2020-02-12 22:23 ` [PATCH v2 1/2] ARM: sun7i: " andrey.lebedev
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2020-02-11  7:20 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-kernel, dri-devel, wens, daniel, linux-arm-kernel

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

Hi,

On Mon, Feb 10, 2020 at 09:56:33PM +0200, Andrey Lebedev wrote:
> A20 SoC (found in Cubieboard 2 among others) requires different LVDS set
> up procedure than A33. Timing controller (tcon) driver only implements
> sun6i-style procedure, that doesn't work on A20 (sun7i).

You're missing your Signed-off-by here.

> The support for such procedure is ported from u-boot and follows u-boot
> naming convention: SUN6I* for sun6i-style procedure, and SUN4I for other
> (which happens to be compatible with A20).

A commit log is mostly about why you're doing a change, this part can
be left out.

> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 91 ++++++++++++++++++++----------
>  drivers/gpu/drm/sun4i/sun4i_tcon.h | 12 ++++
>  2 files changed, 73 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index c81cdce6ed55..78896e907ca9 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -114,46 +114,74 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
>  	}
>  }
>
> +static void sun4i_tcon_lvds_sun6i_enable(struct sun4i_tcon *tcon,
> +					 const struct drm_encoder *encoder) {

This doesn't match the kernel coding style, make sure to run checkpatch.

Also, using something like sun6i_tcon_setup_lvds_phy would be more fit
to what this function is doing.

> +	u8 val;
> +	regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +		     SUN6I_TCON0_LVDS_ANA0_C(2) |
> +		     SUN6I_TCON0_LVDS_ANA0_V(3) |
> +		     SUN6I_TCON0_LVDS_ANA0_PD(2) |
> +		     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
> +	udelay(2);
> +
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_MB,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_MB);
> +	udelay(2);
> +
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
> +
> +	if (sun4i_tcon_get_pixel_depth(encoder) == 18)
> +		val = 7;
> +	else
> +		val = 0xf;
> +
> +	regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
> +			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
> +
> +}
> +
> +static void sun4i_tcon_lvds_sun4i_enable(struct sun4i_tcon *tcon) {

And sun4i_tcon_setup_lvds_phy.

> +	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(2); /* delay at least 1200 ns */
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
> +			   SUN4I_TCON0_LVDS_ANA1_INIT,
> +			   SUN4I_TCON0_LVDS_ANA1_INIT);
> +	udelay(1); /* delay at least 1200 ns */

The delay and your comment don't match.

> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
> +			   SUN4I_TCON0_LVDS_ANA1_UPDATE,
> +			   SUN4I_TCON0_LVDS_ANA1_UPDATE);

You refer to U-Boot in your commit log, but the sequence is not quite
the same, why did you change it?

> +}
> +
> +
>  static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon,
>  				       const struct drm_encoder *encoder,
>  				       bool enabled)
>  {
>  	if (enabled) {
> -		u8 val;
> -
> +		// Enable LVDS interface

There's no need for that comment, it's simple enough :)

>  		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
>  				   SUN4I_TCON0_LVDS_IF_EN,
>  				   SUN4I_TCON0_LVDS_IF_EN);
>
> -		/*
> -		 * As their name suggest, these values only apply to the A31
> -		 * and later SoCs. We'll have to rework this when merging
> -		 * support for the older SoCs.
> -		 */
> -		regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> -			     SUN6I_TCON0_LVDS_ANA0_C(2) |
> -			     SUN6I_TCON0_LVDS_ANA0_V(3) |
> -			     SUN6I_TCON0_LVDS_ANA0_PD(2) |
> -			     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
> -		udelay(2);
> -
> -		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> -				   SUN6I_TCON0_LVDS_ANA0_EN_MB,
> -				   SUN6I_TCON0_LVDS_ANA0_EN_MB);
> -		udelay(2);
> -
> -		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> -				   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
> -				   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
> -
> -		if (sun4i_tcon_get_pixel_depth(encoder) == 18)
> -			val = 7;
> -		else
> -			val = 0xf;
> +		// Perform SoC-specific setup procedure

Ditto.

> +		if (tcon->quirks->sun6i_lvds_init) {
> +			sun4i_tcon_lvds_sun6i_enable(tcon, encoder);
> +		}
> +		else {
> +			sun4i_tcon_lvds_sun4i_enable(tcon);
> +		}
>
> -		regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> -				  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
> -				  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
>  	} else {
>  		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
>  				   SUN4I_TCON0_LVDS_IF_EN, 0);
> @@ -1454,6 +1482,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
>  };
>
>  static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
> +	.supports_lvds		= true,
>  	.has_channel_0		= true,
>  	.has_channel_1		= true,
>  	.dclk_min_div		= 4,
> @@ -1464,11 +1493,13 @@ static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
>  static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
>  	.has_channel_0		= true,
>  	.has_lvds_alt		= true,
> +	.sun6i_lvds_init	= true,

Using a function pointer here (like we're doing with set_mux) would be
more future proof.

>  	.dclk_min_div		= 1,
>  };
>
>  static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = {
>  	.supports_lvds		= true,
> +	.sun6i_lvds_init	= true,
>  	.has_channel_0		= true,
>  	.dclk_min_div		= 1,
>  };
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> index a62ec826ae71..973901c1bee5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> @@ -193,6 +193,13 @@
>  #define SUN4I_TCON_MUX_CTRL_REG			0x200
>
>  #define SUN4I_TCON0_LVDS_ANA0_REG		0x220
> +#define SUN4I_TCON0_LVDS_ANA0_DCHS			BIT(16)
> +#define SUN4I_TCON0_LVDS_ANA0_PD			BIT(20) | BIT(21)
> +#define SUN4I_TCON0_LVDS_ANA0_EN_MB			BIT(22)
> +#define SUN4I_TCON0_LVDS_ANA0_REG_C			BIT(24) | BIT(25)
> +#define SUN4I_TCON0_LVDS_ANA0_REG_V			BIT(26) | BIT(27)
> +#define SUN4I_TCON0_LVDS_ANA0_CK_EN			BIT(29) | BIT(28)
> +
>  #define SUN6I_TCON0_LVDS_ANA0_EN_MB			BIT(31)
>  #define SUN6I_TCON0_LVDS_ANA0_EN_LDO			BIT(30)
>  #define SUN6I_TCON0_LVDS_ANA0_EN_DRVC			BIT(24)
> @@ -201,6 +208,10 @@
>  #define SUN6I_TCON0_LVDS_ANA0_V(x)			(((x) & 3) << 8)
>  #define SUN6I_TCON0_LVDS_ANA0_PD(x)			(((x) & 3) << 4)
>
> +#define SUN4I_TCON0_LVDS_ANA1_REG		0x224
> +#define SUN4I_TCON0_LVDS_ANA1_INIT			(0x1f << 26 | 0x1f << 10)
> +#define SUN4I_TCON0_LVDS_ANA1_UPDATE			(0x1f << 16 | 0x1f << 00)

Having proper defines for those fields would be great too.

Side question, this will need some DT changes too, right?

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH 1/1] Support LVDS output on Allwinner A20
  2020-02-11  7:20 ` Maxime Ripard
@ 2020-02-11 20:48   ` Andrey Lebedev
  2020-02-12 12:53     ` Maxime Ripard
  0 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-11 20:48 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: airlied, linux-kernel, dri-devel, wens, daniel, linux-arm-kernel

Maxime, thanks for your comments. I'll update the patch, but meanwhile,
I have some remarks/questions, see below.

On Tue, Feb 11, 2020 at 08:20:04AM +0100, Maxime Ripard wrote:
> > +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
> > +			   SUN4I_TCON0_LVDS_ANA1_UPDATE,
> > +			   SUN4I_TCON0_LVDS_ANA1_UPDATE);
> 
> You refer to U-Boot in your commit log, but the sequence is not quite
> the same, why did you change it?

I actually had two reference implementations at my hand. One was u-boot
and another - an old (abandoned) branch of Priit Laes [1] (I took the
split-up of u-boot SUNXI_LCDC_LVDS_ANA0 constant from there).

This is an attempt to simplify the sequence, since I noticed that the
same bit was being set to the same register twice [2] and removing that
duplication didn't produce any observable regression. Priit
implementation didn't have that bit set in the end of the sequence
either, so I omitted it. That said, I agree that it could've been a bit
naive on my side, and I can get it back to match u-boot version, if you
feel that might be important.

For the reference the U-Boot code is here: [3]

[1] https://github.com/plaes/linux/commit/cc8c8bab2f2f2752ba6b11632dcd0f41bac249bc#diff-014a76a5007005a7a240825a972b8c7fR127
[2] setbits_le32(&lcdc->lvds_ana0, SUNXI_LCDC_LVDS_ANA0_UPDATE);
[3] https://github.com/ARM-software/u-boot/blob/master/drivers/video/sunxi/lcdc.c#L60

> > +#define SUN4I_TCON0_LVDS_ANA1_REG		0x224
> > +#define SUN4I_TCON0_LVDS_ANA1_INIT			(0x1f << 26 | 0x1f << 10)
> > +#define SUN4I_TCON0_LVDS_ANA1_UPDATE			(0x1f << 16 | 0x1f << 00)
> 
> Having proper defines for those fields would be great too.

If by "proper" you mean "split them up to individual bits", I would
agree, but I can't find any actual hardware reference documentation that
would mention the meaning of these registers.

In both places (u-boot and Priit) these constants are defined the same way. 

I took the liberty to rename ANA1_INIT1 to ANA1_INIT and ANA1_INIT2 to
ANA1_UPDATE to match Priit naming rather than u-boot, as I felt it was
more descriptive. I have no strong opinion here though. 

> Side question, this will need some DT changes too, right?

Hm, I agree. I think it would be reasonable to include LVDS0/1 pins and
sample (but disabled) lvds panel, connected to tcon to
arch/arm/boot/dts/sun7i-a20.dtsi. Does that make sense to you? Would you
expect dts changes in the same patch or separate?

P.S. This is my first patch to the linux kernel, please forgive me my
inexperience.

-- 
Andrey Lebedev aka -.- . -.. -.. . .-.
Software engineer
Homepage: http://lebedev.lt/

_______________________________________________
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] 36+ messages in thread

* Re: [PATCH 1/1] Support LVDS output on Allwinner A20
  2020-02-11 20:48   ` Andrey Lebedev
@ 2020-02-12 12:53     ` Maxime Ripard
  2020-02-12 22:46       ` Andrey Lebedev
  0 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2020-02-12 12:53 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-kernel, dri-devel, wens, daniel, linux-arm-kernel

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

Hi Andrey,

On Tue, Feb 11, 2020 at 10:48:28PM +0200, Andrey Lebedev wrote:
> Maxime, thanks for your comments. I'll update the patch, but meanwhile,
> I have some remarks/questions, see below.
>
> On Tue, Feb 11, 2020 at 08:20:04AM +0100, Maxime Ripard wrote:
> > > +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
> > > +			   SUN4I_TCON0_LVDS_ANA1_UPDATE,
> > > +			   SUN4I_TCON0_LVDS_ANA1_UPDATE);
> >
> > You refer to U-Boot in your commit log, but the sequence is not quite
> > the same, why did you change it?
>
> I actually had two reference implementations at my hand. One was u-boot
> and another - an old (abandoned) branch of Priit Laes [1] (I took the
> split-up of u-boot SUNXI_LCDC_LVDS_ANA0 constant from there).
>
> This is an attempt to simplify the sequence, since I noticed that the
> same bit was being set to the same register twice [2] and removing that
> duplication didn't produce any observable regression. Priit
> implementation didn't have that bit set in the end of the sequence
> either, so I omitted it. That said, I agree that it could've been a bit
> naive on my side, and I can get it back to match u-boot version, if you
> feel that might be important.
>
> For the reference the U-Boot code is here: [3]
>
> [1] https://github.com/plaes/linux/commit/cc8c8bab2f2f2752ba6b11632dcd0f41bac249bc#diff-014a76a5007005a7a240825a972b8c7fR127
> [2] setbits_le32(&lcdc->lvds_ana0, SUNXI_LCDC_LVDS_ANA0_UPDATE);
> [3] https://github.com/ARM-software/u-boot/blob/master/drivers/video/sunxi/lcdc.c#L60

The U-Boot code has been here for a while and we know it's robust by
now, so I'd prefer to be conservative and use it here.

> > > +#define SUN4I_TCON0_LVDS_ANA1_REG		0x224
> > > +#define SUN4I_TCON0_LVDS_ANA1_INIT			(0x1f << 26 | 0x1f << 10)
> > > +#define SUN4I_TCON0_LVDS_ANA1_UPDATE			(0x1f << 16 | 0x1f << 00)
> >
> > Having proper defines for those fields would be great too.
>
> If by "proper" you mean "split them up to individual bits", I would
> agree, but I can't find any actual hardware reference documentation that
> would mention the meaning of these registers.

Of course we don't.. :)

It's fine to leave them as is then

> In both places (u-boot and Priit) these constants are defined the same way.
>
> I took the liberty to rename ANA1_INIT1 to ANA1_INIT and ANA1_INIT2 to
> ANA1_UPDATE to match Priit naming rather than u-boot, as I felt it was
> more descriptive. I have no strong opinion here though.
>
> > Side question, this will need some DT changes too, right?
>
> Hm, I agree. I think it would be reasonable to include LVDS0/1 pins

That, but most importantly, the reset and clocks for the LVDS
block. Also from looking at it, I'm not entirely sure that the TCON1
has a LVDS output, do you have a board when you have been able to test
it?

> and sample (but disabled) lvds panel,

That's good for the sake of the example, but it shouldn't be in the
same patch, it won't be merged.

> connected to tcon to arch/arm/boot/dts/sun7i-a20.dtsi. Does that
> make sense to you? Would you expect dts changes in the same patch or
> separate?
>
> P.S. This is my first patch to the linux kernel, please forgive me my
> inexperience.

You're doing fine so far :)
Maxime

[-- 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] 36+ messages in thread

* [PATCH v2 1/2] ARM: sun7i: Support LVDS output on Allwinner A20
  2020-02-10 19:56 [PATCH 1/1] Support LVDS output on Allwinner A20 Andrey Lebedev
  2020-02-11  7:20 ` Maxime Ripard
@ 2020-02-12 22:23 ` " andrey.lebedev
  2020-02-13  9:32   ` Maxime Ripard
  2020-02-12 22:23 ` [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20 andrey.lebedev
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 36+ messages in thread
From: andrey.lebedev @ 2020-02-12 22:23 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: Andrey Lebedev, linux-sunxi

From: Andrey Lebedev <andrey@lebedev.lt>

A20 SoC (found in Cubieboard 2 among others) requires different LVDS set
up procedure than A33. Timing controller (tcon) driver only implements
sun6i-style procedure, that doesn't work on A20 (sun7i).

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 95 ++++++++++++++++++++----------
 drivers/gpu/drm/sun4i/sun4i_tcon.h | 14 +++++
 2 files changed, 77 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index c81cdce6ed55..e4c605ca685e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -114,46 +114,73 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
 	}
 }
 
+static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
+				      const struct drm_encoder *encoder)
+{
+	u8 val;
+
+	regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+		     SUN6I_TCON0_LVDS_ANA0_C(2) |
+		     SUN6I_TCON0_LVDS_ANA0_V(3) |
+		     SUN6I_TCON0_LVDS_ANA0_PD(2) |
+		     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
+	udelay(2);
+
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN6I_TCON0_LVDS_ANA0_EN_MB,
+			   SUN6I_TCON0_LVDS_ANA0_EN_MB);
+	udelay(2);
+
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
+			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
+
+	if (sun4i_tcon_get_pixel_depth(encoder) == 18)
+		val = 7;
+	else
+		val = 0xf;
+
+	regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
+			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
+
+}
+
+static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
+				      const struct drm_encoder *encoder)
+{
+	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(2); /* delay at least 1200 ns */
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
+			   SUN4I_TCON0_LVDS_ANA1_INIT,
+			   SUN4I_TCON0_LVDS_ANA1_INIT);
+	udelay(1); /* delay at least 120 ns */
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
+			   SUN4I_TCON0_LVDS_ANA1_UPDATE,
+			   SUN4I_TCON0_LVDS_ANA1_UPDATE);
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN4I_TCON0_LVDS_ANA0_EN_MB,
+			   SUN4I_TCON0_LVDS_ANA0_EN_MB);
+}
+
+
 static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon,
 				       const struct drm_encoder *encoder,
 				       bool enabled)
 {
 	if (enabled) {
-		u8 val;
-
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
 				   SUN4I_TCON0_LVDS_IF_EN,
 				   SUN4I_TCON0_LVDS_IF_EN);
-
-		/*
-		 * As their name suggest, these values only apply to the A31
-		 * and later SoCs. We'll have to rework this when merging
-		 * support for the older SoCs.
-		 */
-		regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-			     SUN6I_TCON0_LVDS_ANA0_C(2) |
-			     SUN6I_TCON0_LVDS_ANA0_V(3) |
-			     SUN6I_TCON0_LVDS_ANA0_PD(2) |
-			     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
-		udelay(2);
-
-		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				   SUN6I_TCON0_LVDS_ANA0_EN_MB,
-				   SUN6I_TCON0_LVDS_ANA0_EN_MB);
-		udelay(2);
-
-		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
-				   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
-
-		if (sun4i_tcon_get_pixel_depth(encoder) == 18)
-			val = 7;
-		else
-			val = 0xf;
-
-		regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
-				  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
+		if (tcon->quirks->setup_lvds_phy)
+			tcon->quirks->setup_lvds_phy(tcon, encoder);
 	} else {
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
 				   SUN4I_TCON0_LVDS_IF_EN, 0);
@@ -1454,23 +1481,27 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
 };
 
 static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
+	.supports_lvds		= true,
 	.has_channel_0		= true,
 	.has_channel_1		= true,
 	.dclk_min_div		= 4,
 	/* Same display pipeline structure as A10 */
 	.set_mux		= sun4i_a10_tcon_set_mux,
+	.setup_lvds_phy		= sun4i_tcon_setup_lvds_phy,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
 	.has_channel_0		= true,
 	.has_lvds_alt		= true,
 	.dclk_min_div		= 1,
+	.setup_lvds_phy		= sun6i_tcon_setup_lvds_phy,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = {
 	.supports_lvds		= true,
 	.has_channel_0		= true,
 	.dclk_min_div		= 1,
+	.setup_lvds_phy		= sun6i_tcon_setup_lvds_phy,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = {
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index a62ec826ae71..cfbf4e6c1679 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -193,6 +193,13 @@
 #define SUN4I_TCON_MUX_CTRL_REG			0x200
 
 #define SUN4I_TCON0_LVDS_ANA0_REG		0x220
+#define SUN4I_TCON0_LVDS_ANA0_DCHS			BIT(16)
+#define SUN4I_TCON0_LVDS_ANA0_PD			(BIT(20) | BIT(21))
+#define SUN4I_TCON0_LVDS_ANA0_EN_MB			BIT(22)
+#define SUN4I_TCON0_LVDS_ANA0_REG_C			(BIT(24) | BIT(25))
+#define SUN4I_TCON0_LVDS_ANA0_REG_V			(BIT(26) | BIT(27))
+#define SUN4I_TCON0_LVDS_ANA0_CK_EN			(BIT(29) | BIT(28))
+
 #define SUN6I_TCON0_LVDS_ANA0_EN_MB			BIT(31)
 #define SUN6I_TCON0_LVDS_ANA0_EN_LDO			BIT(30)
 #define SUN6I_TCON0_LVDS_ANA0_EN_DRVC			BIT(24)
@@ -201,6 +208,10 @@
 #define SUN6I_TCON0_LVDS_ANA0_V(x)			(((x) & 3) << 8)
 #define SUN6I_TCON0_LVDS_ANA0_PD(x)			(((x) & 3) << 4)
 
+#define SUN4I_TCON0_LVDS_ANA1_REG		0x224
+#define SUN4I_TCON0_LVDS_ANA1_INIT			(0x1f << 26 | 0x1f << 10)
+#define SUN4I_TCON0_LVDS_ANA1_UPDATE			(0x1f << 16 | 0x1f << 00)
+
 #define SUN4I_TCON1_FILL_CTL_REG		0x300
 #define SUN4I_TCON1_FILL_BEG0_REG		0x304
 #define SUN4I_TCON1_FILL_END0_REG		0x308
@@ -228,6 +239,9 @@ struct sun4i_tcon_quirks {
 
 	/* callback to handle tcon muxing options */
 	int	(*set_mux)(struct sun4i_tcon *, const struct drm_encoder *);
+	/* handler for LVDS setup routine */
+	void	(*setup_lvds_phy)(struct sun4i_tcon *tcon,
+				  const struct drm_encoder *encoder);
 };
 
 struct sun4i_tcon {
-- 
2.20.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] 36+ messages in thread

* [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20
  2020-02-10 19:56 [PATCH 1/1] Support LVDS output on Allwinner A20 Andrey Lebedev
  2020-02-11  7:20 ` Maxime Ripard
  2020-02-12 22:23 ` [PATCH v2 1/2] ARM: sun7i: " andrey.lebedev
@ 2020-02-12 22:23 ` andrey.lebedev
  2020-02-13  9:43   ` Maxime Ripard
  2020-02-13 20:18 ` [PATCH v3 1/3] drm/sun4i: tcon: Introduce LVDS setup routine setting Andrey Lebedev
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 36+ messages in thread
From: andrey.lebedev @ 2020-02-12 22:23 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: Andrey Lebedev, linux-sunxi

From: Andrey Lebedev <andrey@lebedev.lt>

Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and
provide sample LVDS panel, connected to tcon0.

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 45 +++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 92b5be97085d..b05fdf8df32e 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -47,6 +47,7 @@
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/clock/sun7i-a20-ccu.h>
 #include <dt-bindings/reset/sun4i-a10-ccu.h>
+#include <dt-bindings/pinctrl/sun4i-a10.h>
 
 / {
 	interrupt-parent = <&gic>;
@@ -407,8 +408,8 @@
 			compatible = "allwinner,sun7i-a20-tcon";
 			reg = <0x01c0c000 0x1000>;
 			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
-			resets = <&ccu RST_TCON0>;
-			reset-names = "lcd";
+			resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;
+			reset-names = "lcd", "lvds";
 			clocks = <&ccu CLK_AHB_LCD0>,
 				 <&ccu CLK_TCON0_CH0>,
 				 <&ccu CLK_TCON0_CH1>;
@@ -444,6 +445,11 @@
 					#size-cells = <0>;
 					reg = <1>;
 
+					tcon0_out_lvds: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&lvds_in_tcon0>;
+						allwinner,tcon-channel = <0>;
+					};
 					tcon0_out_hdmi: endpoint@1 {
 						reg = <1>;
 						remote-endpoint = <&hdmi_in_tcon0>;
@@ -686,6 +692,19 @@
 			};
 		};
 
+		lvds_panel: panel@1c16500 {
+			compatible = "panel-lvds";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			port {
+				lvds_in_tcon0: endpoint {
+					remote-endpoint = <&tcon0_out_lvds>;
+				};
+			};
+		};
+
 		spi2: spi@1c17000 {
 			compatible = "allwinner,sun4i-a10-spi";
 			reg = <0x01c17000 0x1000>;
@@ -872,7 +891,7 @@
 			gmac_rgmii_pins: gmac-rgmii-pins {
 				pins = "PA0", "PA1", "PA2",
 				       "PA3", "PA4", "PA5", "PA6",
-				        "PA7", "PA8", "PA10",
+					"PA7", "PA8", "PA10",
 				       "PA11", "PA12", "PA13",
 				       "PA15", "PA16";
 				function = "gmac";
@@ -1162,6 +1181,26 @@
 				pins = "PI20", "PI21";
 				function = "uart7";
 			};
+
+			/omit-if-no-ref/
+			lcd_lvds0_pins: lcd_lvds0_pins {
+				allwinner,pins =
+					"PD0", "PD1", "PD2", "PD3", "PD4",
+					"PD5", "PD6", "PD7", "PD8", "PD9";
+				allwinner,function = "lvds0";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			/omit-if-no-ref/
+			lcd_lvds1_pins: lcd_lvds1_pins {
+				allwinner,pins =
+					"PD10", "PD11", "PD12", "PD13", "PD14",
+					"PD15", "PD16", "PD17", "PD18", "PD19";
+				allwinner,function = "lvds1";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
 		};
 
 		timer@1c20c00 {
-- 
2.20.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] 36+ messages in thread

* Re: [PATCH 1/1] Support LVDS output on Allwinner A20
  2020-02-12 12:53     ` Maxime Ripard
@ 2020-02-12 22:46       ` Andrey Lebedev
  2020-02-13  9:24         ` Maxime Ripard
  0 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-12 22:46 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: airlied, linux-kernel, dri-devel, wens, daniel, linux-arm-kernel

On Wed, Feb 12, 2020 at 01:53:45PM +0100, Maxime Ripard wrote:
> > > Side question, this will need some DT changes too, right?
> >
> > Hm, I agree. I think it would be reasonable to include LVDS0/1 pins
> 
> That, but most importantly, the reset and clocks for the LVDS
> block. Also from looking at it, I'm not entirely sure that the TCON1
> has a LVDS output

I also have impression that LVDS is only supported on TCON0, but that's
mostly from this comment in sun4i_lvds.c:

	/* The LVDS encoder can only work with the TCON channel 0 */

> do you have a board when you have been able to test it?

Yes, I have the hardware (Cubieboard 2) at hand, but I cannot change the
any physical connections on it. FWIW, it is https://openvario.org, the
device we are (painfully) trying to upgrade from old kernel-3.4 with
proprietary mali drivers to contemporary software.

> > and sample (but disabled) lvds panel,
> 
> That's good for the sake of the example, but it shouldn't be in the
> same patch, it won't be merged.

I jave just submitted version 2 of the patches - set of 2 patches this
time. Addressed your comments, please take a look.


-- 
Andrey Lebedev aka -.- . -.. -.. . .-.
Software engineer
Homepage: http://lebedev.lt/

_______________________________________________
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] 36+ messages in thread

* Re: [PATCH 1/1] Support LVDS output on Allwinner A20
  2020-02-12 22:46       ` Andrey Lebedev
@ 2020-02-13  9:24         ` Maxime Ripard
  2020-02-13 18:11           ` Andrey Lebedev
  0 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2020-02-13  9:24 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-kernel, dri-devel, wens, daniel, linux-arm-kernel

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

On Thu, Feb 13, 2020 at 12:46:53AM +0200, Andrey Lebedev wrote:
> On Wed, Feb 12, 2020 at 01:53:45PM +0100, Maxime Ripard wrote:
> > > > Side question, this will need some DT changes too, right?
> > >
> > > Hm, I agree. I think it would be reasonable to include LVDS0/1 pins
> >
> > That, but most importantly, the reset and clocks for the LVDS
> > block. Also from looking at it, I'm not entirely sure that the TCON1
> > has a LVDS output
>
> I also have impression that LVDS is only supported on TCON0, but that's
> mostly from this comment in sun4i_lvds.c:
>
> 	/* The LVDS encoder can only work with the TCON channel 0 */

No, that's a separate thing.

Internally the TCON has two channels, one connected to panels type of
display (LVDS, Parallel, etc), the second one connected to TV outputs
(HDMI, composite).

But then, on some SoCs like the A20, you have two TCON's too. As far
as I could see, only the first TCON can use LVDS, but I'm not
definitive.

Allwinner seems to allow panels to only be tied to TCON0 in the BSP,
so I guess we can assume that.

> > do you have a board when you have been able to test it?
>
> Yes, I have the hardware (Cubieboard 2) at hand, but I cannot change the
> any physical connections on it. FWIW, it is https://openvario.org, the
> device we are (painfully) trying to upgrade from old kernel-3.4 with
> proprietary mali drivers to contemporary software.

What painpoints do you have?

> > > and sample (but disabled) lvds panel,
> >
> > That's good for the sake of the example, but it shouldn't be in the
> > same patch, it won't be merged.
>
> I jave just submitted version 2 of the patches - set of 2 patches this
> time. Addressed your comments, please take a look.

I will, thanks!
Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH v2 1/2] ARM: sun7i: Support LVDS output on Allwinner A20
  2020-02-12 22:23 ` [PATCH v2 1/2] ARM: sun7i: " andrey.lebedev
@ 2020-02-13  9:32   ` Maxime Ripard
  0 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2020-02-13  9:32 UTC (permalink / raw)
  To: andrey.lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

Hi,

The prefix of your title should be "drm/sun4i: tcon:" instead of "ARM:
sun7i:"

The latter would be if you were modifying files under arch/arm, but
you're modifying files in (drivers/gpu/)drm/sun4i :)

On Thu, Feb 13, 2020 at 12:23:55AM +0200, andrey.lebedev@gmail.com wrote:
> From: Andrey Lebedev <andrey@lebedev.lt>
>
> A20 SoC (found in Cubieboard 2 among others) requires different LVDS set
> up procedure than A33. Timing controller (tcon) driver only implements
> sun6i-style procedure, that doesn't work on A20 (sun7i).
>
> Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 95 ++++++++++++++++++++----------
>  drivers/gpu/drm/sun4i/sun4i_tcon.h | 14 +++++
>  2 files changed, 77 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index c81cdce6ed55..e4c605ca685e 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -114,46 +114,73 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
>  	}
>  }
>
> +static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
> +				      const struct drm_encoder *encoder)
> +{
> +	u8 val;
> +
> +	regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +		     SUN6I_TCON0_LVDS_ANA0_C(2) |
> +		     SUN6I_TCON0_LVDS_ANA0_V(3) |
> +		     SUN6I_TCON0_LVDS_ANA0_PD(2) |
> +		     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
> +	udelay(2);
> +
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_MB,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_MB);
> +	udelay(2);
> +
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
> +
> +	if (sun4i_tcon_get_pixel_depth(encoder) == 18)
> +		val = 7;
> +	else
> +		val = 0xf;
> +
> +	regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
> +			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
> +
> +}
> +
> +static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
> +				      const struct drm_encoder *encoder)
> +{
> +	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(2); /* delay at least 1200 ns */
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
> +			   SUN4I_TCON0_LVDS_ANA1_INIT,
> +			   SUN4I_TCON0_LVDS_ANA1_INIT);
> +	udelay(1); /* delay at least 120 ns */
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
> +			   SUN4I_TCON0_LVDS_ANA1_UPDATE,
> +			   SUN4I_TCON0_LVDS_ANA1_UPDATE);
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			   SUN4I_TCON0_LVDS_ANA0_EN_MB,
> +			   SUN4I_TCON0_LVDS_ANA0_EN_MB);
> +}
> +
> +

Patches should contain only one logical change, so ideally this should
be two patches: one to create the function pointer and fill it for the
A31 style setup, the other one to add support for the A20.

Also, you should have only a single line between the two functions and
the second should come before the first (alphabetical ordering).

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20
  2020-02-12 22:23 ` [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20 andrey.lebedev
@ 2020-02-13  9:43   ` Maxime Ripard
  2020-02-13 20:08     ` Andrey Lebedev
  0 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2020-02-13  9:43 UTC (permalink / raw)
  To: andrey.lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

On Thu, Feb 13, 2020 at 12:23:57AM +0200, andrey.lebedev@gmail.com wrote:
> From: Andrey Lebedev <andrey@lebedev.lt>
>
> Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and
> provide sample LVDS panel, connected to tcon0.
>
> Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>

And this prefix should be ARM: dts: sun7i ;)

> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi | 45 +++++++++++++++++++++++++++++---
>  1 file changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 92b5be97085d..b05fdf8df32e 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -47,6 +47,7 @@
>  #include <dt-bindings/dma/sun4i-a10.h>
>  #include <dt-bindings/clock/sun7i-a20-ccu.h>
>  #include <dt-bindings/reset/sun4i-a10-ccu.h>
> +#include <dt-bindings/pinctrl/sun4i-a10.h>
>
>  / {
>  	interrupt-parent = <&gic>;
> @@ -407,8 +408,8 @@
>  			compatible = "allwinner,sun7i-a20-tcon";
>  			reg = <0x01c0c000 0x1000>;
>  			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
> -			resets = <&ccu RST_TCON0>;
> -			reset-names = "lcd";
> +			resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;
> +			reset-names = "lcd", "lvds";
>  			clocks = <&ccu CLK_AHB_LCD0>,
>  				 <&ccu CLK_TCON0_CH0>,
>  				 <&ccu CLK_TCON0_CH1>;
> @@ -444,6 +445,11 @@
>  					#size-cells = <0>;
>  					reg = <1>;
>
> +					tcon0_out_lvds: endpoint@0 {
> +						reg = <0>;
> +						remote-endpoint = <&lvds_in_tcon0>;
> +						allwinner,tcon-channel = <0>;
> +					};

A new line here would be nice

>  					tcon0_out_hdmi: endpoint@1 {
>  						reg = <1>;
>  						remote-endpoint = <&hdmi_in_tcon0>;
> @@ -686,6 +692,19 @@
>  			};
>  		};
>
> +		lvds_panel: panel@1c16500 {
> +			compatible = "panel-lvds";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +
> +			port {
> +				lvds_in_tcon0: endpoint {
> +					remote-endpoint = <&tcon0_out_lvds>;
> +				};
> +			};
> +		};
> +

There's no point in creating that panel.

>  		spi2: spi@1c17000 {
>  			compatible = "allwinner,sun4i-a10-spi";
>  			reg = <0x01c17000 0x1000>;
> @@ -872,7 +891,7 @@
>  			gmac_rgmii_pins: gmac-rgmii-pins {
>  				pins = "PA0", "PA1", "PA2",
>  				       "PA3", "PA4", "PA5", "PA6",
> -				        "PA7", "PA8", "PA10",
> +					"PA7", "PA8", "PA10",
>  				       "PA11", "PA12", "PA13",
>  				       "PA15", "PA16";
>  				function = "gmac";
> @@ -1162,6 +1181,26 @@
>  				pins = "PI20", "PI21";
>  				function = "uart7";
>  			};
> +
> +			/omit-if-no-ref/
> +			lcd_lvds0_pins: lcd_lvds0_pins {

underscores in the node names will create a dtc warning at
compilation, you should use lcd-lvds0-pins instead.

> +				allwinner,pins =
> +					"PD0", "PD1", "PD2", "PD3", "PD4",
> +					"PD5", "PD6", "PD7", "PD8", "PD9";
> +				allwinner,function = "lvds0";
> +				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;

Those properties are deprecated and should be replaced by pins and
functions. allwinner,drive and allwinner,pull are at their default
values and can be dropped.

This will create a spurious warning message for TCON1, since we
adjusted the driver to tell it supports LVDS, but there's no LVDS
reset line, so we need to make it finer grained.

Maybe adding a tcon0 / tcon1 compatible? Chen-Yu, any thought?

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH 1/1] Support LVDS output on Allwinner A20
  2020-02-13  9:24         ` Maxime Ripard
@ 2020-02-13 18:11           ` Andrey Lebedev
  2020-02-14  7:58             ` Maxime Ripard
  0 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-13 18:11 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: airlied, linux-kernel, dri-devel, wens, daniel, linux-arm-kernel

On Thu, Feb 13, 2020 at 10:24:33AM +0100, Maxime Ripard wrote:
> > > do you have a board when you have been able to test it?
> >
> > Yes, I have the hardware (Cubieboard 2) at hand, but I cannot change the
> > any physical connections on it. FWIW, it is https://openvario.org, the
> > device we are (painfully) trying to upgrade from old kernel-3.4 with
> > proprietary mali drivers to contemporary software.
> 
> What painpoints do you have?

So, even though proprietary mali drivers worked well for us, they seem
to hold us back to old kernel-3.4, and it started to get harder to avoid
various compatibility issues with newer software. Since there seemed to
be a good progress with OSS lima driver lately, we decided to try to
replace mali with lima. And that naturally pulled to switch to DRM/KMS.

The first painpoint is this LVDS support problem: after a week of trial
and error, I discovered that support was simply not there. But it seemed
that not that much was actually missing and after couple of evenings
deep into debugging, here we are.

Another one is the screen rotation: the device is installed into the
glider' instrument panel, and some pilots prefer it in portrait
orientation.  Under old mali setup, all that (seemingly) was required
was setting "fbcon=rotate:n" boot arg, and both linux console and
graphical app (https://xcsoar.org/) rotated "automagically". With new
DRM/KMS setup, only console is rotated, all graphical apps seem to see
the screen in its "natural" landscape orientation. Do you know if there
is any way to leverage DMS/KMS infrastructure to make screen appear
rotated for userspace apps (writing to /sys/class/graphics/fb0/rotate
didn't work)?  There's of course a plan B to teach the app to rotate its
output, but that leads to problem number 3.

And the 3rd outstanding problem is that app doesn't really work under
lima module and lima mesa driver. It starts, but renders a black window.
I haven't dug deeply into this yet, but it is possible that some opengl
API isn't supported in OSS drivers yet (even though app only renders 2d
graphics).

Hopefully that wasn't too much complaining for you :) If you have any
insight on possible causes or attack vectors on any of these, that would
help a lot. Also, please feel free to correct any of false assumptions I
make above, I'm happy to learn more about this.


-- 
Andrey Lebedev aka -.- . -.. -.. . .-.
Software engineer
Homepage: http://lebedev.lt/

_______________________________________________
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] 36+ messages in thread

* Re: [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20
  2020-02-13  9:43   ` Maxime Ripard
@ 2020-02-13 20:08     ` Andrey Lebedev
  2020-02-14  7:52       ` Maxime Ripard
  0 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-13 20:08 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

On Thu, Feb 13, 2020 at 10:43:04AM +0100, Maxime Ripard wrote:
> On Thu, Feb 13, 2020 at 12:23:57AM +0200, andrey.lebedev@gmail.com wrote:
> > From: Andrey Lebedev <andrey@lebedev.lt>
> >
> > Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and
> > provide sample LVDS panel, connected to tcon0.
> >
> > Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> 
> And this prefix should be ARM: dts: sun7i ;)

Ah, thanks, I think I've got the pattern now!

> > +			/omit-if-no-ref/
> > +			lcd_lvds0_pins: lcd_lvds0_pins {
> 
> underscores in the node names will create a dtc warning at
> compilation, you should use lcd-lvds0-pins instead.

You're right, I wasn't following the naming convention here. dtc doesn't
produce any warning on this though. Fixed that anyway.

> This will create a spurious warning message for TCON1, since we
> adjusted the driver to tell it supports LVDS, but there's no LVDS
> reset line, so we need to make it finer grained.

Yes, I can attribute two of the messages in my dmesg log [1] to this
("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon
1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they
can be confusing to someone.

I'd need some pointers on how to deal with that though (if we want to do
it in this scope).

[1] excerpt from kernel boot log:

[    4.890975] sun4i-drm display-engine: bound 1e00000.display-frontend (ops sun4i_frontend_driver_exit [sun4i_frontend])
[    4.902032] sun4i-drm display-engine: bound 1e20000.display-frontend (ops sun4i_frontend_driver_exit [sun4i_frontend])
[    4.913467] sun4i-drm display-engine: bound 1e60000.display-backend (ops sun4i_backend_ops [sun4i_backend])
[    4.923806] sun4i-drm display-engine: bound 1e40000.display-backend (ops sun4i_backend_ops [sun4i_backend])
[    4.934451] sun4i-drm display-engine: bound 1c0c000.lcd-controller (ops sun4i_tcon_platform_driver_exit [sun4i_tcon])
[    4.945254] sun4i-tcon 1c0d000.lcd-controller: Missing LVDS properties, Please upgrade your DT
[    4.953935] sun4i-tcon 1c0d000.lcd-controller: LVDS output disabled
[    4.960857] sun4i-drm display-engine: No panel or bridge found... RGB output disabled
[    4.968845] sun4i-drm display-engine: bound 1c0d000.lcd-controller (ops sun4i_tcon_platform_driver_exit [sun4i_tcon])
[    5.080874] sun4i-drm display-engine: bound 1c16000.hdmi (ops sun4i_hdmi_driver_exit [sun4i_drm_hdmi])
[    5.110087] [drm] Initialized sun4i-drm 1.0.0 20150629 for display-engine on minor 0
[    5.763064] sun4i-drm display-engine: fb0: sun4i-drmdrmfb frame buffer device


-- 
Andrey Lebedev aka -.- . -.. -.. . .-.
Software engineer
Homepage: http://lebedev.lt/

_______________________________________________
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] 36+ messages in thread

* [PATCH v3 1/3] drm/sun4i: tcon: Introduce LVDS setup routine setting
  2020-02-10 19:56 [PATCH 1/1] Support LVDS output on Allwinner A20 Andrey Lebedev
                   ` (2 preceding siblings ...)
  2020-02-12 22:23 ` [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20 andrey.lebedev
@ 2020-02-13 20:18 ` Andrey Lebedev
  2020-02-13 20:18 ` [PATCH v3 2/3] drm/sun4i: tcon: Support LVDS output on Allwinner A20 Andrey Lebedev
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-13 20:18 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: Andrey Lebedev, linux-sunxi

From: Andrey Lebedev <andrey@lebedev.lt>

Different sunxi flavors require slightly different sequence for enabling
LVDS output. This allows to differentiate between them.

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 68 ++++++++++++++++--------------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  3 ++
 2 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index c81cdce6ed55..cc6b05ca2c69 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -114,46 +114,48 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
 	}
 }
 
+static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
+				      const struct drm_encoder *encoder)
+{
+	u8 val;
+
+	regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+		     SUN6I_TCON0_LVDS_ANA0_C(2) |
+		     SUN6I_TCON0_LVDS_ANA0_V(3) |
+		     SUN6I_TCON0_LVDS_ANA0_PD(2) |
+		     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
+	udelay(2);
+
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN6I_TCON0_LVDS_ANA0_EN_MB,
+			   SUN6I_TCON0_LVDS_ANA0_EN_MB);
+	udelay(2);
+
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
+			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
+
+	if (sun4i_tcon_get_pixel_depth(encoder) == 18)
+		val = 7;
+	else
+		val = 0xf;
+
+	regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
+			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
+
+}
+
 static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon,
 				       const struct drm_encoder *encoder,
 				       bool enabled)
 {
 	if (enabled) {
-		u8 val;
-
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
 				   SUN4I_TCON0_LVDS_IF_EN,
 				   SUN4I_TCON0_LVDS_IF_EN);
-
-		/*
-		 * As their name suggest, these values only apply to the A31
-		 * and later SoCs. We'll have to rework this when merging
-		 * support for the older SoCs.
-		 */
-		regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-			     SUN6I_TCON0_LVDS_ANA0_C(2) |
-			     SUN6I_TCON0_LVDS_ANA0_V(3) |
-			     SUN6I_TCON0_LVDS_ANA0_PD(2) |
-			     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
-		udelay(2);
-
-		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				   SUN6I_TCON0_LVDS_ANA0_EN_MB,
-				   SUN6I_TCON0_LVDS_ANA0_EN_MB);
-		udelay(2);
-
-		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
-				   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
-
-		if (sun4i_tcon_get_pixel_depth(encoder) == 18)
-			val = 7;
-		else
-			val = 0xf;
-
-		regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
-				  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
+		if (tcon->quirks->setup_lvds_phy)
+			tcon->quirks->setup_lvds_phy(tcon, encoder);
 	} else {
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
 				   SUN4I_TCON0_LVDS_IF_EN, 0);
@@ -1465,12 +1467,14 @@ static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
 	.has_channel_0		= true,
 	.has_lvds_alt		= true,
 	.dclk_min_div		= 1,
+	.setup_lvds_phy		= sun6i_tcon_setup_lvds_phy,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = {
 	.supports_lvds		= true,
 	.has_channel_0		= true,
 	.dclk_min_div		= 1,
+	.setup_lvds_phy		= sun6i_tcon_setup_lvds_phy,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = {
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index a62ec826ae71..2974e59ef9f2 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -228,6 +228,9 @@ struct sun4i_tcon_quirks {
 
 	/* callback to handle tcon muxing options */
 	int	(*set_mux)(struct sun4i_tcon *, const struct drm_encoder *);
+	/* handler for LVDS setup routine */
+	void	(*setup_lvds_phy)(struct sun4i_tcon *tcon,
+				  const struct drm_encoder *encoder);
 };
 
 struct sun4i_tcon {
-- 
2.20.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] 36+ messages in thread

* [PATCH v3 2/3] drm/sun4i: tcon: Support LVDS output on Allwinner A20
  2020-02-10 19:56 [PATCH 1/1] Support LVDS output on Allwinner A20 Andrey Lebedev
                   ` (3 preceding siblings ...)
  2020-02-13 20:18 ` [PATCH v3 1/3] drm/sun4i: tcon: Introduce LVDS setup routine setting Andrey Lebedev
@ 2020-02-13 20:18 ` Andrey Lebedev
  2020-02-13 20:18 ` [PATCH v3 3/3] ARM: dts: sun7i: Add LVDS panel support on A20 Andrey Lebedev
  2020-02-19 18:08 ` PATCH v4 Andrey Lebedev
  6 siblings, 0 replies; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-13 20:18 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: Andrey Lebedev, linux-sunxi

From: Andrey Lebedev <andrey@lebedev.lt>

A20 SoC (found in Cubieboard 2 among others) requires different LVDS set
up procedure than A33. Timing controller (tcon) driver only implements
sun6i-style procedure, that doesn't work on A20 (sun7i).

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_tcon.h | 11 +++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index cc6b05ca2c69..800a9bd86112 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -114,6 +114,30 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
 	}
 }
 
+static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
+				      const struct drm_encoder *encoder)
+{
+	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(2); /* delay at least 1200 ns */
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
+			   SUN4I_TCON0_LVDS_ANA1_INIT,
+			   SUN4I_TCON0_LVDS_ANA1_INIT);
+	udelay(1); /* delay at least 120 ns */
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
+			   SUN4I_TCON0_LVDS_ANA1_UPDATE,
+			   SUN4I_TCON0_LVDS_ANA1_UPDATE);
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN4I_TCON0_LVDS_ANA0_EN_MB,
+			   SUN4I_TCON0_LVDS_ANA0_EN_MB);
+}
+
 static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
 				      const struct drm_encoder *encoder)
 {
@@ -1456,11 +1480,13 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
 };
 
 static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
+	.supports_lvds		= true,
 	.has_channel_0		= true,
 	.has_channel_1		= true,
 	.dclk_min_div		= 4,
 	/* Same display pipeline structure as A10 */
 	.set_mux		= sun4i_a10_tcon_set_mux,
+	.setup_lvds_phy		= sun4i_tcon_setup_lvds_phy,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index 2974e59ef9f2..cfbf4e6c1679 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -193,6 +193,13 @@
 #define SUN4I_TCON_MUX_CTRL_REG			0x200
 
 #define SUN4I_TCON0_LVDS_ANA0_REG		0x220
+#define SUN4I_TCON0_LVDS_ANA0_DCHS			BIT(16)
+#define SUN4I_TCON0_LVDS_ANA0_PD			(BIT(20) | BIT(21))
+#define SUN4I_TCON0_LVDS_ANA0_EN_MB			BIT(22)
+#define SUN4I_TCON0_LVDS_ANA0_REG_C			(BIT(24) | BIT(25))
+#define SUN4I_TCON0_LVDS_ANA0_REG_V			(BIT(26) | BIT(27))
+#define SUN4I_TCON0_LVDS_ANA0_CK_EN			(BIT(29) | BIT(28))
+
 #define SUN6I_TCON0_LVDS_ANA0_EN_MB			BIT(31)
 #define SUN6I_TCON0_LVDS_ANA0_EN_LDO			BIT(30)
 #define SUN6I_TCON0_LVDS_ANA0_EN_DRVC			BIT(24)
@@ -201,6 +208,10 @@
 #define SUN6I_TCON0_LVDS_ANA0_V(x)			(((x) & 3) << 8)
 #define SUN6I_TCON0_LVDS_ANA0_PD(x)			(((x) & 3) << 4)
 
+#define SUN4I_TCON0_LVDS_ANA1_REG		0x224
+#define SUN4I_TCON0_LVDS_ANA1_INIT			(0x1f << 26 | 0x1f << 10)
+#define SUN4I_TCON0_LVDS_ANA1_UPDATE			(0x1f << 16 | 0x1f << 00)
+
 #define SUN4I_TCON1_FILL_CTL_REG		0x300
 #define SUN4I_TCON1_FILL_BEG0_REG		0x304
 #define SUN4I_TCON1_FILL_END0_REG		0x308
-- 
2.20.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] 36+ messages in thread

* [PATCH v3 3/3] ARM: dts: sun7i: Add LVDS panel support on A20
  2020-02-10 19:56 [PATCH 1/1] Support LVDS output on Allwinner A20 Andrey Lebedev
                   ` (4 preceding siblings ...)
  2020-02-13 20:18 ` [PATCH v3 2/3] drm/sun4i: tcon: Support LVDS output on Allwinner A20 Andrey Lebedev
@ 2020-02-13 20:18 ` Andrey Lebedev
  2020-02-14  8:55   ` Maxime Ripard
  2020-02-19 18:08 ` PATCH v4 Andrey Lebedev
  6 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-13 20:18 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: Andrey Lebedev, linux-sunxi

From: Andrey Lebedev <andrey@lebedev.lt>

Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and
provide sample LVDS panel, connected to tcon0.

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 92b5be97085d..3b3c366a2bee 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -47,6 +47,7 @@
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/clock/sun7i-a20-ccu.h>
 #include <dt-bindings/reset/sun4i-a10-ccu.h>
+#include <dt-bindings/pinctrl/sun4i-a10.h>
 
 / {
 	interrupt-parent = <&gic>;
@@ -407,8 +408,8 @@
 			compatible = "allwinner,sun7i-a20-tcon";
 			reg = <0x01c0c000 0x1000>;
 			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
-			resets = <&ccu RST_TCON0>;
-			reset-names = "lcd";
+			resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;
+			reset-names = "lcd", "lvds";
 			clocks = <&ccu CLK_AHB_LCD0>,
 				 <&ccu CLK_TCON0_CH0>,
 				 <&ccu CLK_TCON0_CH1>;
@@ -444,6 +445,11 @@
 					#size-cells = <0>;
 					reg = <1>;
 
+					tcon0_out_lvds: endpoint@0 {
+						reg = <0>;
+						allwinner,tcon-channel = <0>;
+					};
+
 					tcon0_out_hdmi: endpoint@1 {
 						reg = <1>;
 						remote-endpoint = <&hdmi_in_tcon0>;
@@ -1162,6 +1168,24 @@
 				pins = "PI20", "PI21";
 				function = "uart7";
 			};
+
+			/omit-if-no-ref/
+			lcd_lvds0_pins: lcd-lvds0-pins {
+				pins =
+					"PD0", "PD1", "PD2", "PD3", "PD4",
+					"PD5", "PD6", "PD7", "PD8", "PD9";
+				function = "lvds0";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+			};
+
+			/omit-if-no-ref/
+			lcd_lvds1_pins: lcd-lvds1-pins {
+				pins =
+					"PD10", "PD11", "PD12", "PD13", "PD14",
+					"PD15", "PD16", "PD17", "PD18", "PD19";
+				function = "lvds1";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+			};
 		};
 
 		timer@1c20c00 {
-- 
2.20.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] 36+ messages in thread

* Re: [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20
  2020-02-13 20:08     ` Andrey Lebedev
@ 2020-02-14  7:52       ` Maxime Ripard
  2020-02-14  8:43         ` Andrey Lebedev
  0 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2020-02-14  7:52 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

Hi,

On Thu, Feb 13, 2020 at 10:08:23PM +0200, Andrey Lebedev wrote:
> On Thu, Feb 13, 2020 at 10:43:04AM +0100, Maxime Ripard wrote:
> > On Thu, Feb 13, 2020 at 12:23:57AM +0200, andrey.lebedev@gmail.com wrote:
> > > From: Andrey Lebedev <andrey@lebedev.lt>
> > >
> > > Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and
> > > provide sample LVDS panel, connected to tcon0.
> > >
> > > Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> >
> > And this prefix should be ARM: dts: sun7i ;)
>
> Ah, thanks, I think I've got the pattern now!
>
> > > +			/omit-if-no-ref/
> > > +			lcd_lvds0_pins: lcd_lvds0_pins {
> >
> > underscores in the node names will create a dtc warning at
> > compilation, you should use lcd-lvds0-pins instead.

It's silenced by default, but you'll get them with W=1

> You're right, I wasn't following the naming convention here. dtc doesn't
> produce any warning on this though. Fixed that anyway.
>
> > This will create a spurious warning message for TCON1, since we
> > adjusted the driver to tell it supports LVDS, but there's no LVDS
> > reset line, so we need to make it finer grained.
>
> Yes, I can attribute two of the messages in my dmesg log [1] to this
> ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon
> 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they
> can be confusing to someone.
>
> I'd need some pointers on how to deal with that though (if we want to do
> it in this scope).

Like I was mentionning, you could introduce a new compatible for each
TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH 1/1] Support LVDS output on Allwinner A20
  2020-02-13 18:11           ` Andrey Lebedev
@ 2020-02-14  7:58             ` Maxime Ripard
  0 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2020-02-14  7:58 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-kernel, dri-devel, wens, daniel, linux-arm-kernel

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

On Thu, Feb 13, 2020 at 08:11:10PM +0200, Andrey Lebedev wrote:
> On Thu, Feb 13, 2020 at 10:24:33AM +0100, Maxime Ripard wrote:
> > > > do you have a board when you have been able to test it?
> > >
> > > Yes, I have the hardware (Cubieboard 2) at hand, but I cannot change the
> > > any physical connections on it. FWIW, it is https://openvario.org, the
> > > device we are (painfully) trying to upgrade from old kernel-3.4 with
> > > proprietary mali drivers to contemporary software.
> >
> > What painpoints do you have?
>
> So, even though proprietary mali drivers worked well for us, they seem
> to hold us back to old kernel-3.4, and it started to get harder to avoid
> various compatibility issues with newer software. Since there seemed to
> be a good progress with OSS lima driver lately, we decided to try to
> replace mali with lima. And that naturally pulled to switch to DRM/KMS.

You can use the proprietary mali drivers with mainline too, but yeah
it's in maintainance mode these days and you should go for lima if
you're (re)starting from scratch.

> The first painpoint is this LVDS support problem: after a week of trial
> and error, I discovered that support was simply not there. But it seemed
> that not that much was actually missing and after couple of evenings
> deep into debugging, here we are.
>
> Another one is the screen rotation: the device is installed into the
> glider' instrument panel, and some pilots prefer it in portrait
> orientation.  Under old mali setup, all that (seemingly) was required
> was setting "fbcon=rotate:n" boot arg, and both linux console and
> graphical app (https://xcsoar.org/) rotated "automagically". With new
> DRM/KMS setup, only console is rotated, all graphical apps seem to see
> the screen in its "natural" landscape orientation. Do you know if there
> is any way to leverage DMS/KMS infrastructure to make screen appear
> rotated for userspace apps (writing to /sys/class/graphics/fb0/rotate
> didn't work)?  There's of course a plan B to teach the app to rotate its
> output, but that leads to problem number 3.

There's a rotation property that can be set by whatever you're using
on top of KMS, DRM_MODE_ROTATE_*, but I'm not sure the driver supports
it at the moment.

> And the 3rd outstanding problem is that app doesn't really work under
> lima module and lima mesa driver. It starts, but renders a black window.
> I haven't dug deeply into this yet, but it is possible that some opengl
> API isn't supported in OSS drivers yet (even though app only renders 2d
> graphics).

I have no idea on this one though, sorry :/

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20
  2020-02-14  7:52       ` Maxime Ripard
@ 2020-02-14  8:43         ` Andrey Lebedev
  2020-02-14  8:53           ` Maxime Ripard
  0 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-14  8:43 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

On Fri, Feb 14, 2020 at 08:52:18AM +0100, Maxime Ripard wrote:
> > > This will create a spurious warning message for TCON1, since we
> > > adjusted the driver to tell it supports LVDS, but there's no LVDS
> > > reset line, so we need to make it finer grained.
> >
> > Yes, I can attribute two of the messages in my dmesg log [1] to this
> > ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon
> > 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they
> > can be confusing to someone.
> >
> > I'd need some pointers on how to deal with that though (if we want to do
> > it in this scope).
> 
> Like I was mentionning, you could introduce a new compatible for each
> TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0

Can you give me an idea how that compatible might look like?

		tcon0: lcd-controller@1c0c000 {
			compatible = "allwinner,sun7i-a20-tcon", "allwinner,lvds";

or

		tcon0: lcd-controller@1c0c000 {
			compatible = "allwinner,sun7i-a20-tcon", "allwinner,tcon0";

? Or something completely different?


-- 
Andrey Lebedev aka -.- . -.. -.. . .-.
Software engineer
Homepage: http://lebedev.lt/

_______________________________________________
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] 36+ messages in thread

* Re: [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20
  2020-02-14  8:43         ` Andrey Lebedev
@ 2020-02-14  8:53           ` Maxime Ripard
  2020-02-14 21:32             ` Andrey Lebedev
  0 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2020-02-14  8:53 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

On Fri, Feb 14, 2020 at 10:43:58AM +0200, Andrey Lebedev wrote:
> On Fri, Feb 14, 2020 at 08:52:18AM +0100, Maxime Ripard wrote:
> > > > This will create a spurious warning message for TCON1, since we
> > > > adjusted the driver to tell it supports LVDS, but there's no LVDS
> > > > reset line, so we need to make it finer grained.
> > >
> > > Yes, I can attribute two of the messages in my dmesg log [1] to this
> > > ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon
> > > 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they
> > > can be confusing to someone.
> > >
> > > I'd need some pointers on how to deal with that though (if we want to do
> > > it in this scope).
> >
> > Like I was mentionning, you could introduce a new compatible for each
> > TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0
>
> Can you give me an idea how that compatible might look like?
>
> 		tcon0: lcd-controller@1c0c000 {
> 			compatible = "allwinner,sun7i-a20-tcon", "allwinner,lvds";
>
> or
>
> 		tcon0: lcd-controller@1c0c000 {
> 			compatible = "allwinner,sun7i-a20-tcon", "allwinner,tcon0";
>
> ? Or something completely different?

Something like

&tcon0 {
    compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon";
};

&tcon1 {
    compatible = "allwinner,sun7i-a20-tcon1", "allwinner,sun7i-a20-tcon";
};

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH v3 3/3] ARM: dts: sun7i: Add LVDS panel support on A20
  2020-02-13 20:18 ` [PATCH v3 3/3] ARM: dts: sun7i: Add LVDS panel support on A20 Andrey Lebedev
@ 2020-02-14  8:55   ` Maxime Ripard
  0 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2020-02-14  8:55 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

On Thu, Feb 13, 2020 at 10:18:57PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <andrey@lebedev.lt>
>
> Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and
> provide sample LVDS panel, connected to tcon0.
>
> Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 92b5be97085d..3b3c366a2bee 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -47,6 +47,7 @@
>  #include <dt-bindings/dma/sun4i-a10.h>
>  #include <dt-bindings/clock/sun7i-a20-ccu.h>
>  #include <dt-bindings/reset/sun4i-a10-ccu.h>
> +#include <dt-bindings/pinctrl/sun4i-a10.h>
>
>  / {
>  	interrupt-parent = <&gic>;
> @@ -407,8 +408,8 @@
>  			compatible = "allwinner,sun7i-a20-tcon";
>  			reg = <0x01c0c000 0x1000>;
>  			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
> -			resets = <&ccu RST_TCON0>;
> -			reset-names = "lcd";
> +			resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;
> +			reset-names = "lcd", "lvds";
>  			clocks = <&ccu CLK_AHB_LCD0>,
>  				 <&ccu CLK_TCON0_CH0>,
>  				 <&ccu CLK_TCON0_CH1>;
> @@ -444,6 +445,11 @@
>  					#size-cells = <0>;
>  					reg = <1>;
>
> +					tcon0_out_lvds: endpoint@0 {
> +						reg = <0>;
> +						allwinner,tcon-channel = <0>;
> +					};
> +
>  					tcon0_out_hdmi: endpoint@1 {
>  						reg = <1>;
>  						remote-endpoint = <&hdmi_in_tcon0>;
> @@ -1162,6 +1168,24 @@
>  				pins = "PI20", "PI21";
>  				function = "uart7";
>  			};
> +
> +			/omit-if-no-ref/
> +			lcd_lvds0_pins: lcd-lvds0-pins {

The nodes here should be ordered by alphabetical order

> +				pins =

I'm not sure why you need a new line here

> +					"PD0", "PD1", "PD2", "PD3", "PD4",
> +					"PD5", "PD6", "PD7", "PD8", "PD9";
> +				function = "lvds0";
> +				allwinner,drive = <SUN4I_PINCTRL_10_MA>;

And allwinner,drive is also deprecated and at its default value anyway

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20
  2020-02-14  8:53           ` Maxime Ripard
@ 2020-02-14 21:32             ` Andrey Lebedev
  2020-02-17 17:51               ` Maxime Ripard
  0 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-14 21:32 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

On Fri, Feb 14, 2020 at 09:53:51AM +0100, Maxime Ripard wrote:
> On Fri, Feb 14, 2020 at 10:43:58AM +0200, Andrey Lebedev wrote:
> > On Fri, Feb 14, 2020 at 08:52:18AM +0100, Maxime Ripard wrote:
> > > > > This will create a spurious warning message for TCON1, since we
> > > > > adjusted the driver to tell it supports LVDS, but there's no LVDS
> > > > > reset line, so we need to make it finer grained.
> > > >
> > > > Yes, I can attribute two of the messages in my dmesg log [1] to this
> > > > ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon
> > > > 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they
> > > > can be confusing to someone.
> > > >
> > > > I'd need some pointers on how to deal with that though (if we want to do
> > > > it in this scope).
> > >
> > > Like I was mentionning, you could introduce a new compatible for each
> > > TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0
> >
> > Can you give me an idea how that compatible might look like?
> >
> > 		tcon0: lcd-controller@1c0c000 {
> > 			compatible = "allwinner,sun7i-a20-tcon", "allwinner,lvds";
> >
> > or
> >
> > 		tcon0: lcd-controller@1c0c000 {
> > 			compatible = "allwinner,sun7i-a20-tcon", "allwinner,tcon0";
> >
> > ? Or something completely different?
> 
> Something like
> 
> &tcon0 {
>     compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon";
> };
> 
> &tcon1 {
>     compatible = "allwinner,sun7i-a20-tcon1", "allwinner,sun7i-a20-tcon";
> };
> 

Hi Maxime, here is what I came up with, please take a look. If the
approach is right, I'll split it up and include into the patch set.

From f3e45c958a9551a52ac26435785bdb572e54d8db Mon Sep 17 00:00:00 2001
From: Andrey Lebedev <andrey@lebedev.lt>
Date: Fri, 14 Feb 2020 23:21:59 +0200
Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20

This allows to avoid warnings about reset line not provided for tcon1.

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 arch/arm/boot/dts/sun7i-a20.dtsi   |  2 +-
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 +++++++++++++++++++++-
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  2 ++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 3b3c366a2bee..bab59fc4d9b1 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -405,7 +405,7 @@
 		};
 
 		tcon0: lcd-controller@1c0c000 {
-			compatible = "allwinner,sun7i-a20-tcon";
+			compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon";
 			reg = <0x01c0c000 0x1000>;
 			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
 			resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 800a9bd86112..cb2040aec436 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -1107,6 +1107,25 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
 	return sun4i_tcon_find_engine_traverse(drv, node, 0);
 }
 
+/*
+ * Check if given tcon supports LVDS
+ *
+ * Some of the sunxi SoC variants contain several timing controllers, but only
+ * one of them can be used to drive LVDS screen. In this case such tcon is
+ * identified in respective quirks struct: lvds_compatible_tcon property will
+ * hold "compatible" string of the tcon, that supports LVDS.
+ *
+ * If lvds_compatible_tcon is not set, all tcons are considered capable of
+ * driving LVDS.
+ */
+static bool sun4i_tcon_lvds_compat(struct device *dev, struct sun4i_tcon *tcon)
+{
+	if (tcon->quirks->lvds_compatible_tcon == NULL)
+		return true;
+	return of_device_is_compatible(dev->of_node,
+	                               tcon->quirks->lvds_compatible_tcon);
+}
+
 static int sun4i_tcon_bind(struct device *dev, struct device *master,
 			   void *data)
 {
@@ -1161,7 +1180,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
-	if (tcon->quirks->supports_lvds) {
+	if (tcon->quirks->supports_lvds && sun4i_tcon_lvds_compat(dev, tcon)) {
 		/*
 		 * This can only be made optional since we've had DT
 		 * nodes without the LVDS reset properties.
@@ -1481,6 +1500,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
 
 static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
 	.supports_lvds		= true,
+	.lvds_compatible_tcon	= "allwinner,sun7i-a20-tcon0",
 	.has_channel_0		= true,
 	.has_channel_1		= true,
 	.dclk_min_div		= 4,
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index cfbf4e6c1679..bc87d28ee341 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -235,6 +235,8 @@ struct sun4i_tcon_quirks {
 	bool	needs_de_be_mux; /* sun6i needs mux to select backend */
 	bool    needs_edp_reset; /* a80 edp reset needed for tcon0 access */
 	bool	supports_lvds;   /* Does the TCON support an LVDS output? */
+	/* "compatible" string of TCON that exclusively supports LVDS */
+	const char *lvds_compatible_tcon;
 	u8	dclk_min_div;	/* minimum divider for TCON0 DCLK */
 
 	/* callback to handle tcon muxing options */
-- 
2.20.1


-- 
Andrey Lebedev aka -.- . -.. -.. . .-.
Software engineer
Homepage: http://lebedev.lt/

_______________________________________________
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] 36+ messages in thread

* Re: [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20
  2020-02-14 21:32             ` Andrey Lebedev
@ 2020-02-17 17:51               ` Maxime Ripard
  2020-02-18 17:50                 ` Andrey Lebedev
  0 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2020-02-17 17:51 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

Hi Andrey,

On Fri, Feb 14, 2020 at 11:32:31PM +0200, Andrey Lebedev wrote:
> On Fri, Feb 14, 2020 at 09:53:51AM +0100, Maxime Ripard wrote:
> > On Fri, Feb 14, 2020 at 10:43:58AM +0200, Andrey Lebedev wrote:
> > > On Fri, Feb 14, 2020 at 08:52:18AM +0100, Maxime Ripard wrote:
> > > > > > This will create a spurious warning message for TCON1, since we
> > > > > > adjusted the driver to tell it supports LVDS, but there's no LVDS
> > > > > > reset line, so we need to make it finer grained.
> > > > >
> > > > > Yes, I can attribute two of the messages in my dmesg log [1] to this
> > > > > ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon
> > > > > 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they
> > > > > can be confusing to someone.
> > > > >
> > > > > I'd need some pointers on how to deal with that though (if we want to do
> > > > > it in this scope).
> > > >
> > > > Like I was mentionning, you could introduce a new compatible for each
> > > > TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0
> > >
> > > Can you give me an idea how that compatible might look like?
> > >
> > > 		tcon0: lcd-controller@1c0c000 {
> > > 			compatible = "allwinner,sun7i-a20-tcon", "allwinner,lvds";
> > >
> > > or
> > >
> > > 		tcon0: lcd-controller@1c0c000 {
> > > 			compatible = "allwinner,sun7i-a20-tcon", "allwinner,tcon0";
> > >
> > > ? Or something completely different?
> >
> > Something like
> >
> > &tcon0 {
> >     compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon";
> > };
> >
> > &tcon1 {
> >     compatible = "allwinner,sun7i-a20-tcon1", "allwinner,sun7i-a20-tcon";
> > };
> >
>
> Hi Maxime, here is what I came up with, please take a look. If the
> approach is right, I'll split it up and include into the patch set.
>
> From f3e45c958a9551a52ac26435785bdb572e54d8db Mon Sep 17 00:00:00 2001
> From: Andrey Lebedev <andrey@lebedev.lt>
> Date: Fri, 14 Feb 2020 23:21:59 +0200
> Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20
>
> This allows to avoid warnings about reset line not provided for tcon1.
>
> Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi   |  2 +-
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 +++++++++++++++++++++-
>  drivers/gpu/drm/sun4i/sun4i_tcon.h |  2 ++
>  3 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 3b3c366a2bee..bab59fc4d9b1 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -405,7 +405,7 @@
>  		};
>
>  		tcon0: lcd-controller@1c0c000 {
> -			compatible = "allwinner,sun7i-a20-tcon";
> +			compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon";

That's correct

>  			reg = <0x01c0c000 0x1000>;
>  			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
>  			resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 800a9bd86112..cb2040aec436 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -1107,6 +1107,25 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
>  	return sun4i_tcon_find_engine_traverse(drv, node, 0);
>  }
>
> +/*
> + * Check if given tcon supports LVDS
> + *
> + * Some of the sunxi SoC variants contain several timing controllers, but only
> + * one of them can be used to drive LVDS screen. In this case such tcon is
> + * identified in respective quirks struct: lvds_compatible_tcon property will
> + * hold "compatible" string of the tcon, that supports LVDS.
> + *
> + * If lvds_compatible_tcon is not set, all tcons are considered capable of
> + * driving LVDS.
> + */
> +static bool sun4i_tcon_lvds_compat(struct device *dev, struct sun4i_tcon *tcon)
> +{
> +	if (tcon->quirks->lvds_compatible_tcon == NULL)
> +		return true;
> +	return of_device_is_compatible(dev->of_node,
> +	                               tcon->quirks->lvds_compatible_tcon);
> +}
> +
>  static int sun4i_tcon_bind(struct device *dev, struct device *master,
>  			   void *data)
>  {
> @@ -1161,7 +1180,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
>  		return ret;
>  	}
>
> -	if (tcon->quirks->supports_lvds) {
> +	if (tcon->quirks->supports_lvds && sun4i_tcon_lvds_compat(dev, tcon)) {
>  		/*
>  		 * This can only be made optional since we've had DT
>  		 * nodes without the LVDS reset properties.
> @@ -1481,6 +1500,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
>
>  static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
>  	.supports_lvds		= true,
> +	.lvds_compatible_tcon	= "allwinner,sun7i-a20-tcon0",
>  	.has_channel_0		= true,
>  	.has_channel_1		= true,
>  	.dclk_min_div		= 4,
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> index cfbf4e6c1679..bc87d28ee341 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks {
>  	bool	needs_de_be_mux; /* sun6i needs mux to select backend */
>  	bool    needs_edp_reset; /* a80 edp reset needed for tcon0 access */
>  	bool	supports_lvds;   /* Does the TCON support an LVDS output? */
> +	/* "compatible" string of TCON that exclusively supports LVDS */
> +	const char *lvds_compatible_tcon;

However this is far more complicated than needed, you can simply add a
new quirks structure associated to the tcon0 compatible in
sun4i_tcon_of_table, and that will do

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20
  2020-02-17 17:51               ` Maxime Ripard
@ 2020-02-18 17:50                 ` Andrey Lebedev
  2020-02-19 12:06                   ` Maxime Ripard
  0 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-18 17:50 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

On Mon, Feb 17, 2020 at 06:51:35PM +0100, Maxime Ripard wrote:
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> > index cfbf4e6c1679..bc87d28ee341 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> > @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks {
> >  	bool	needs_de_be_mux; /* sun6i needs mux to select backend */
> >  	bool    needs_edp_reset; /* a80 edp reset needed for tcon0 access */
> >  	bool	supports_lvds;   /* Does the TCON support an LVDS output? */
> > +	/* "compatible" string of TCON that exclusively supports LVDS */
> > +	const char *lvds_compatible_tcon;
> 
> However this is far more complicated than needed, you can simply add a
> new quirks structure associated to the tcon0 compatible in
> sun4i_tcon_of_table, and that will do

Aha! Does this look good to you?

From 4ad2978e404c63d4cca1b7890bc5bdd4d1e8965d Mon Sep 17 00:00:00 2001
From: Andrey Lebedev <andrey@lebedev.lt>
Date: Tue, 18 Feb 2020 19:47:33 +0200
Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20

This allows to avoid warnings about reset line not provided for tcon1.

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 arch/arm/boot/dts/sun7i-a20.dtsi   |  6 ++++--
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 14 ++++++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index dc4f3f249aee..d50263c1ca9a 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -405,7 +405,8 @@
 		};
 
 		tcon0: lcd-controller@1c0c000 {
-			compatible = "allwinner,sun7i-a20-tcon";
+			compatible = "allwinner,sun7i-a20-tcon0",
+				     "allwinner,sun7i-a20-tcon";
 			reg = <0x01c0c000 0x1000>;
 			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
 			resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;
@@ -460,7 +461,8 @@
 		};
 
 		tcon1: lcd-controller@1c0d000 {
-			compatible = "allwinner,sun7i-a20-tcon";
+			compatible = "allwinner,sun7i-a20-tcon1",
+				     "allwinner,sun7i-a20-tcon";
 			reg = <0x01c0d000 0x1000>;
 			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
 			resets = <&ccu RST_TCON1>;
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 800a9bd86112..d9605d331010 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -1479,7 +1479,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
 	.dclk_min_div		= 1,
 };
 
-static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
+static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = {
 	.supports_lvds		= true,
 	.has_channel_0		= true,
 	.has_channel_1		= true,
@@ -1489,6 +1489,15 @@ static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
 	.setup_lvds_phy		= sun4i_tcon_setup_lvds_phy,
 };
 
+static const struct sun4i_tcon_quirks sun7i_a20_tcon1_quirks = {
+	.supports_lvds		= false,
+	.has_channel_0		= true,
+	.has_channel_1		= true,
+	.dclk_min_div		= 4,
+	/* Same display pipeline structure as A10 */
+	.set_mux		= sun4i_a10_tcon_set_mux,
+};
+
 static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
 	.has_channel_0		= true,
 	.has_lvds_alt		= true,
@@ -1534,7 +1543,8 @@ const struct of_device_id sun4i_tcon_of_table[] = {
 	{ .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks },
 	{ .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks },
 	{ .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks },
-	{ .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks },
+	{ .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_tcon0_quirks },
+	{ .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_tcon1_quirks },
 	{ .compatible = "allwinner,sun8i-a23-tcon", .data = &sun8i_a33_quirks },
 	{ .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks },
 	{ .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data = &sun8i_a83t_lcd_quirks },
-- 
2.20.1


-- 
Andrey Lebedev aka -.- . -.. -.. . .-.
Software engineer
Homepage: http://lebedev.lt/

_______________________________________________
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] 36+ messages in thread

* Re: [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20
  2020-02-18 17:50                 ` Andrey Lebedev
@ 2020-02-19 12:06                   ` Maxime Ripard
  0 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2020-02-19 12:06 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

On Tue, Feb 18, 2020 at 07:50:33PM +0200, Andrey Lebedev wrote:
> On Mon, Feb 17, 2020 at 06:51:35PM +0100, Maxime Ripard wrote:
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> > > index cfbf4e6c1679..bc87d28ee341 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> > > @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks {
> > >  	bool	needs_de_be_mux; /* sun6i needs mux to select backend */
> > >  	bool    needs_edp_reset; /* a80 edp reset needed for tcon0 access */
> > >  	bool	supports_lvds;   /* Does the TCON support an LVDS output? */
> > > +	/* "compatible" string of TCON that exclusively supports LVDS */
> > > +	const char *lvds_compatible_tcon;
> >
> > However this is far more complicated than needed, you can simply add a
> > new quirks structure associated to the tcon0 compatible in
> > sun4i_tcon_of_table, and that will do
>
> Aha! Does this look good to you?
>
> From 4ad2978e404c63d4cca1b7890bc5bdd4d1e8965d Mon Sep 17 00:00:00 2001
> From: Andrey Lebedev <andrey@lebedev.lt>
> Date: Tue, 18 Feb 2020 19:47:33 +0200
> Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20
>
> This allows to avoid warnings about reset line not provided for tcon1.
>
> Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi   |  6 ++++--
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 14 ++++++++++++--
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index dc4f3f249aee..d50263c1ca9a 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -405,7 +405,8 @@
>  		};
>
>  		tcon0: lcd-controller@1c0c000 {
> -			compatible = "allwinner,sun7i-a20-tcon";
> +			compatible = "allwinner,sun7i-a20-tcon0",
> +				     "allwinner,sun7i-a20-tcon";
>  			reg = <0x01c0c000 0x1000>;
>  			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
>  			resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;
> @@ -460,7 +461,8 @@
>  		};
>
>  		tcon1: lcd-controller@1c0d000 {
> -			compatible = "allwinner,sun7i-a20-tcon";
> +			compatible = "allwinner,sun7i-a20-tcon1",
> +				     "allwinner,sun7i-a20-tcon";
>  			reg = <0x01c0d000 0x1000>;
>  			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
>  			resets = <&ccu RST_TCON1>;

This needs to be in a separate patch

> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 800a9bd86112..d9605d331010 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -1479,7 +1479,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
>  	.dclk_min_div		= 1,
>  };
>
> -static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
> +static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = {
>  	.supports_lvds		= true,
>  	.has_channel_0		= true,
>  	.has_channel_1		= true,
> @@ -1489,6 +1489,15 @@ static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
>  	.setup_lvds_phy		= sun4i_tcon_setup_lvds_phy,
>  };
>
> +static const struct sun4i_tcon_quirks sun7i_a20_tcon1_quirks = {
> +	.supports_lvds		= false,
> +	.has_channel_0		= true,
> +	.has_channel_1		= true,
> +	.dclk_min_div		= 4,
> +	/* Same display pipeline structure as A10 */
> +	.set_mux		= sun4i_a10_tcon_set_mux,
> +};
> +
>  static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
>  	.has_channel_0		= true,
>  	.has_lvds_alt		= true,
> @@ -1534,7 +1543,8 @@ const struct of_device_id sun4i_tcon_of_table[] = {
>  	{ .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks },
>  	{ .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks },
>  	{ .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks },
> -	{ .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks },
> +	{ .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_tcon0_quirks },
> +	{ .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_tcon1_quirks },
>  	{ .compatible = "allwinner,sun8i-a23-tcon", .data = &sun8i_a33_quirks },
>  	{ .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks },
>  	{ .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data = &sun8i_a83t_lcd_quirks },

And we need to support older DT as well, so you shouldn't remove the
older TCON compatible and its structure. Make sure to document the new
compatibles too.

Maxime

[-- 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] 36+ messages in thread

* PATCH v4
  2020-02-10 19:56 [PATCH 1/1] Support LVDS output on Allwinner A20 Andrey Lebedev
                   ` (5 preceding siblings ...)
  2020-02-13 20:18 ` [PATCH v3 3/3] ARM: dts: sun7i: Add LVDS panel support on A20 Andrey Lebedev
@ 2020-02-19 18:08 ` Andrey Lebedev
  2020-02-19 18:08   ` [PATCH 1/5] drm/sun4i: tcon: Introduce LVDS setup routine setting Andrey Lebedev
                     ` (4 more replies)
  6 siblings, 5 replies; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-19 18:08 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: linux-sunxi

Address all outstanding review comments.

Maxime, please confirm I've got "document the new
compatibles" part right.



_______________________________________________
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] 36+ messages in thread

* [PATCH 1/5] drm/sun4i: tcon: Introduce LVDS setup routine setting
  2020-02-19 18:08 ` PATCH v4 Andrey Lebedev
@ 2020-02-19 18:08   ` Andrey Lebedev
  2020-02-20 17:21     ` Maxime Ripard
  2020-02-19 18:08   ` [PATCH 2/5] drm/sun4i: tcon: Separate quirks for tcon0 and tcon1 on A20 Andrey Lebedev
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-19 18:08 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: Andrey Lebedev, linux-sunxi

From: Andrey Lebedev <andrey@lebedev.lt>

Different sunxi flavors require slightly different sequence for enabling
LVDS output. This allows to differentiate between them.

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 68 ++++++++++++++++--------------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  3 ++
 2 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index c81cdce6ed55..cc6b05ca2c69 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -114,46 +114,48 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
 	}
 }
 
+static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
+				      const struct drm_encoder *encoder)
+{
+	u8 val;
+
+	regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+		     SUN6I_TCON0_LVDS_ANA0_C(2) |
+		     SUN6I_TCON0_LVDS_ANA0_V(3) |
+		     SUN6I_TCON0_LVDS_ANA0_PD(2) |
+		     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
+	udelay(2);
+
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN6I_TCON0_LVDS_ANA0_EN_MB,
+			   SUN6I_TCON0_LVDS_ANA0_EN_MB);
+	udelay(2);
+
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
+			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
+
+	if (sun4i_tcon_get_pixel_depth(encoder) == 18)
+		val = 7;
+	else
+		val = 0xf;
+
+	regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
+			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
+
+}
+
 static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon,
 				       const struct drm_encoder *encoder,
 				       bool enabled)
 {
 	if (enabled) {
-		u8 val;
-
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
 				   SUN4I_TCON0_LVDS_IF_EN,
 				   SUN4I_TCON0_LVDS_IF_EN);
-
-		/*
-		 * As their name suggest, these values only apply to the A31
-		 * and later SoCs. We'll have to rework this when merging
-		 * support for the older SoCs.
-		 */
-		regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-			     SUN6I_TCON0_LVDS_ANA0_C(2) |
-			     SUN6I_TCON0_LVDS_ANA0_V(3) |
-			     SUN6I_TCON0_LVDS_ANA0_PD(2) |
-			     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
-		udelay(2);
-
-		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				   SUN6I_TCON0_LVDS_ANA0_EN_MB,
-				   SUN6I_TCON0_LVDS_ANA0_EN_MB);
-		udelay(2);
-
-		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
-				   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
-
-		if (sun4i_tcon_get_pixel_depth(encoder) == 18)
-			val = 7;
-		else
-			val = 0xf;
-
-		regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
-				  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
-				  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
+		if (tcon->quirks->setup_lvds_phy)
+			tcon->quirks->setup_lvds_phy(tcon, encoder);
 	} else {
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG,
 				   SUN4I_TCON0_LVDS_IF_EN, 0);
@@ -1465,12 +1467,14 @@ static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
 	.has_channel_0		= true,
 	.has_lvds_alt		= true,
 	.dclk_min_div		= 1,
+	.setup_lvds_phy		= sun6i_tcon_setup_lvds_phy,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = {
 	.supports_lvds		= true,
 	.has_channel_0		= true,
 	.dclk_min_div		= 1,
+	.setup_lvds_phy		= sun6i_tcon_setup_lvds_phy,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = {
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index a62ec826ae71..2974e59ef9f2 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -228,6 +228,9 @@ struct sun4i_tcon_quirks {
 
 	/* callback to handle tcon muxing options */
 	int	(*set_mux)(struct sun4i_tcon *, const struct drm_encoder *);
+	/* handler for LVDS setup routine */
+	void	(*setup_lvds_phy)(struct sun4i_tcon *tcon,
+				  const struct drm_encoder *encoder);
 };
 
 struct sun4i_tcon {
-- 
2.20.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] 36+ messages in thread

* [PATCH 2/5] drm/sun4i: tcon: Separate quirks for tcon0 and tcon1 on A20
  2020-02-19 18:08 ` PATCH v4 Andrey Lebedev
  2020-02-19 18:08   ` [PATCH 1/5] drm/sun4i: tcon: Introduce LVDS setup routine setting Andrey Lebedev
@ 2020-02-19 18:08   ` Andrey Lebedev
  2020-02-20 17:22     ` Maxime Ripard
  2020-02-19 18:08   ` [PATCH 3/5] ARM: dts: sun7i: Add LVDS panel support " Andrey Lebedev
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-19 18:08 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: Andrey Lebedev, linux-sunxi

From: Andrey Lebedev <andrey@lebedev.lt>

Timing controllers on A20 are not equivalent: tcon0 on A20 supports
LVDS output and tcon1 does not. Separate the capabilities by
introducing independent set of quirks for each of the tcons.

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index cc6b05ca2c69..b7234eef3c7b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -1508,6 +1508,8 @@ const struct of_device_id sun4i_tcon_of_table[] = {
 	{ .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks },
 	{ .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks },
 	{ .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks },
+	{ .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_quirks },
+	{ .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_quirks },
 	{ .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks },
 	{ .compatible = "allwinner,sun8i-a23-tcon", .data = &sun8i_a33_quirks },
 	{ .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks },
-- 
2.20.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] 36+ messages in thread

* [PATCH 3/5] ARM: dts: sun7i: Add LVDS panel support on A20
  2020-02-19 18:08 ` PATCH v4 Andrey Lebedev
  2020-02-19 18:08   ` [PATCH 1/5] drm/sun4i: tcon: Introduce LVDS setup routine setting Andrey Lebedev
  2020-02-19 18:08   ` [PATCH 2/5] drm/sun4i: tcon: Separate quirks for tcon0 and tcon1 on A20 Andrey Lebedev
@ 2020-02-19 18:08   ` " Andrey Lebedev
  2020-02-20 17:23     ` Maxime Ripard
  2020-02-19 18:08   ` [PATCH 4/5] dt-bindings: display: sun4i: New compatibles for A20 tcons Andrey Lebedev
  2020-02-19 18:08   ` [PATCH 5/5] drm/sun4i: tcon: Support LVDS output on Allwinner A20 Andrey Lebedev
  4 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-19 18:08 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: Andrey Lebedev, linux-sunxi

From: Andrey Lebedev <andrey@lebedev.lt>

Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and
provide sample LVDS panel, connected to tcon0.

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 92b5be97085d..d50263c1ca9a 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -47,6 +47,7 @@
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/clock/sun7i-a20-ccu.h>
 #include <dt-bindings/reset/sun4i-a10-ccu.h>
+#include <dt-bindings/pinctrl/sun4i-a10.h>
 
 / {
 	interrupt-parent = <&gic>;
@@ -404,11 +405,12 @@
 		};
 
 		tcon0: lcd-controller@1c0c000 {
-			compatible = "allwinner,sun7i-a20-tcon";
+			compatible = "allwinner,sun7i-a20-tcon0",
+				     "allwinner,sun7i-a20-tcon";
 			reg = <0x01c0c000 0x1000>;
 			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
-			resets = <&ccu RST_TCON0>;
-			reset-names = "lcd";
+			resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;
+			reset-names = "lcd", "lvds";
 			clocks = <&ccu CLK_AHB_LCD0>,
 				 <&ccu CLK_TCON0_CH0>,
 				 <&ccu CLK_TCON0_CH1>;
@@ -444,6 +446,11 @@
 					#size-cells = <0>;
 					reg = <1>;
 
+					tcon0_out_lvds: endpoint@0 {
+						reg = <0>;
+						allwinner,tcon-channel = <0>;
+					};
+
 					tcon0_out_hdmi: endpoint@1 {
 						reg = <1>;
 						remote-endpoint = <&hdmi_in_tcon0>;
@@ -454,7 +461,8 @@
 		};
 
 		tcon1: lcd-controller@1c0d000 {
-			compatible = "allwinner,sun7i-a20-tcon";
+			compatible = "allwinner,sun7i-a20-tcon1",
+				     "allwinner,sun7i-a20-tcon";
 			reg = <0x01c0d000 0x1000>;
 			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
 			resets = <&ccu RST_TCON1>;
@@ -931,6 +939,20 @@
 				function = "ir1";
 			};
 
+			/omit-if-no-ref/
+			lcd_lvds0_pins: lcd-lvds0-pins {
+				pins = "PD0", "PD1", "PD2", "PD3", "PD4",
+				       "PD5", "PD6", "PD7", "PD8", "PD9";
+				function = "lvds0";
+			};
+
+			/omit-if-no-ref/
+			lcd_lvds1_pins: lcd-lvds1-pins {
+				pins = "PD10", "PD11", "PD12", "PD13", "PD14",
+				       "PD15", "PD16", "PD17", "PD18", "PD19";
+				function = "lvds1";
+			};
+
 			/omit-if-no-ref/
 			mmc0_pins: mmc0-pins {
 				pins = "PF0", "PF1", "PF2",
-- 
2.20.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] 36+ messages in thread

* [PATCH 4/5] dt-bindings: display: sun4i: New compatibles for A20 tcons
  2020-02-19 18:08 ` PATCH v4 Andrey Lebedev
                     ` (2 preceding siblings ...)
  2020-02-19 18:08   ` [PATCH 3/5] ARM: dts: sun7i: Add LVDS panel support " Andrey Lebedev
@ 2020-02-19 18:08   ` Andrey Lebedev
  2020-02-20 17:24     ` Maxime Ripard
  2020-02-19 18:08   ` [PATCH 5/5] drm/sun4i: tcon: Support LVDS output on Allwinner A20 Andrey Lebedev
  4 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-19 18:08 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: Andrey Lebedev, linux-sunxi

From: Andrey Lebedev <andrey@lebedev.lt>

Document new compatibles used to differentiate between timing
controllers on A20 (sun7i)

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 .../bindings/display/allwinner,sun4i-a10-tcon.yaml          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
index 86ad617d2327..c0f6bb16fa34 100644
--- a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
+++ b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
@@ -46,6 +46,12 @@ properties:
           - allwinner,sun50i-h6-tcon-tv
         - const: allwinner,sun8i-a83t-tcon-tv
 
+      - items:
+        - enum:
+          - allwinner,sun7i-a20-tcon0
+          - allwinner,sun7i-a20-tcon1
+        - const: allwinner,sun7i-a20-tcon
+
   reg:
     maxItems: 1
 
-- 
2.20.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] 36+ messages in thread

* [PATCH 5/5] drm/sun4i: tcon: Support LVDS output on Allwinner A20
  2020-02-19 18:08 ` PATCH v4 Andrey Lebedev
                     ` (3 preceding siblings ...)
  2020-02-19 18:08   ` [PATCH 4/5] dt-bindings: display: sun4i: New compatibles for A20 tcons Andrey Lebedev
@ 2020-02-19 18:08   ` Andrey Lebedev
  2020-02-20 17:25     ` Maxime Ripard
  4 siblings, 1 reply; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-19 18:08 UTC (permalink / raw)
  To: mripard, wens, airlied, daniel, dri-devel, linux-arm-kernel,
	linux-kernel
  Cc: Andrey Lebedev, linux-sunxi

From: Andrey Lebedev <andrey@lebedev.lt>

A20 SoC (found in Cubieboard 2 among others) requires different LVDS set
up procedure than A33. Timing controller (tcon) driver only implements
sun6i-style procedure, that doesn't work on A20 (sun7i).

Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 37 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/sun4i/sun4i_tcon.h | 11 +++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index b7234eef3c7b..09ee6e8c6914 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -114,6 +114,30 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
 	}
 }
 
+static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
+				      const struct drm_encoder *encoder)
+{
+	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(2); /* delay at least 1200 ns */
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
+			   SUN4I_TCON0_LVDS_ANA1_INIT,
+			   SUN4I_TCON0_LVDS_ANA1_INIT);
+	udelay(1); /* delay at least 120 ns */
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
+			   SUN4I_TCON0_LVDS_ANA1_UPDATE,
+			   SUN4I_TCON0_LVDS_ANA1_UPDATE);
+	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
+			   SUN4I_TCON0_LVDS_ANA0_EN_MB,
+			   SUN4I_TCON0_LVDS_ANA0_EN_MB);
+}
+
 static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
 				      const struct drm_encoder *encoder)
 {
@@ -1455,7 +1479,18 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
 	.dclk_min_div		= 1,
 };
 
+static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = {
+	.supports_lvds		= true,
+	.has_channel_0		= true,
+	.has_channel_1		= true,
+	.dclk_min_div		= 4,
+	/* Same display pipeline structure as A10 */
+	.set_mux		= sun4i_a10_tcon_set_mux,
+	.setup_lvds_phy		= sun4i_tcon_setup_lvds_phy,
+};
+
 static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
+	.supports_lvds		= false,
 	.has_channel_0		= true,
 	.has_channel_1		= true,
 	.dclk_min_div		= 4,
@@ -1508,7 +1543,7 @@ const struct of_device_id sun4i_tcon_of_table[] = {
 	{ .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks },
 	{ .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks },
 	{ .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks },
-	{ .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_quirks },
+	{ .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_tcon0_quirks },
 	{ .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_quirks },
 	{ .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks },
 	{ .compatible = "allwinner,sun8i-a23-tcon", .data = &sun8i_a33_quirks },
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index 2974e59ef9f2..cfbf4e6c1679 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -193,6 +193,13 @@
 #define SUN4I_TCON_MUX_CTRL_REG			0x200
 
 #define SUN4I_TCON0_LVDS_ANA0_REG		0x220
+#define SUN4I_TCON0_LVDS_ANA0_DCHS			BIT(16)
+#define SUN4I_TCON0_LVDS_ANA0_PD			(BIT(20) | BIT(21))
+#define SUN4I_TCON0_LVDS_ANA0_EN_MB			BIT(22)
+#define SUN4I_TCON0_LVDS_ANA0_REG_C			(BIT(24) | BIT(25))
+#define SUN4I_TCON0_LVDS_ANA0_REG_V			(BIT(26) | BIT(27))
+#define SUN4I_TCON0_LVDS_ANA0_CK_EN			(BIT(29) | BIT(28))
+
 #define SUN6I_TCON0_LVDS_ANA0_EN_MB			BIT(31)
 #define SUN6I_TCON0_LVDS_ANA0_EN_LDO			BIT(30)
 #define SUN6I_TCON0_LVDS_ANA0_EN_DRVC			BIT(24)
@@ -201,6 +208,10 @@
 #define SUN6I_TCON0_LVDS_ANA0_V(x)			(((x) & 3) << 8)
 #define SUN6I_TCON0_LVDS_ANA0_PD(x)			(((x) & 3) << 4)
 
+#define SUN4I_TCON0_LVDS_ANA1_REG		0x224
+#define SUN4I_TCON0_LVDS_ANA1_INIT			(0x1f << 26 | 0x1f << 10)
+#define SUN4I_TCON0_LVDS_ANA1_UPDATE			(0x1f << 16 | 0x1f << 00)
+
 #define SUN4I_TCON1_FILL_CTL_REG		0x300
 #define SUN4I_TCON1_FILL_BEG0_REG		0x304
 #define SUN4I_TCON1_FILL_END0_REG		0x308
-- 
2.20.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] 36+ messages in thread

* Re: [PATCH 1/5] drm/sun4i: tcon: Introduce LVDS setup routine setting
  2020-02-19 18:08   ` [PATCH 1/5] drm/sun4i: tcon: Introduce LVDS setup routine setting Andrey Lebedev
@ 2020-02-20 17:21     ` Maxime Ripard
  2020-02-20 18:19       ` Andrey Lebedev
  0 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2020-02-20 17:21 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

On Wed, Feb 19, 2020 at 08:08:54PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <andrey@lebedev.lt>
>
> Different sunxi flavors require slightly different sequence for enabling
> LVDS output. This allows to differentiate between them.
>
> Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 68 ++++++++++++++++--------------
>  drivers/gpu/drm/sun4i/sun4i_tcon.h |  3 ++
>  2 files changed, 39 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index c81cdce6ed55..cc6b05ca2c69 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -114,46 +114,48 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
>  	}
>  }
>
> +static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
> +				      const struct drm_encoder *encoder)
> +{
> +	u8 val;
> +
> +	regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +		     SUN6I_TCON0_LVDS_ANA0_C(2) |
> +		     SUN6I_TCON0_LVDS_ANA0_V(3) |
> +		     SUN6I_TCON0_LVDS_ANA0_PD(2) |
> +		     SUN6I_TCON0_LVDS_ANA0_EN_LDO);
> +	udelay(2);
> +
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_MB,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_MB);
> +	udelay(2);
> +
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC,
> +			   SUN6I_TCON0_LVDS_ANA0_EN_DRVC);
> +
> +	if (sun4i_tcon_get_pixel_depth(encoder) == 18)
> +		val = 7;
> +	else
> +		val = 0xf;
> +
> +	regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
> +			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
> +
> +}
> +

There's an extra blank line here that was reported by checkpatch. I've
fixed it up while applying.

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH 2/5] drm/sun4i: tcon: Separate quirks for tcon0 and tcon1 on A20
  2020-02-19 18:08   ` [PATCH 2/5] drm/sun4i: tcon: Separate quirks for tcon0 and tcon1 on A20 Andrey Lebedev
@ 2020-02-20 17:22     ` Maxime Ripard
  0 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2020-02-20 17:22 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

On Wed, Feb 19, 2020 at 08:08:55PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <andrey@lebedev.lt>
>
> Timing controllers on A20 are not equivalent: tcon0 on A20 supports
> LVDS output and tcon1 does not. Separate the capabilities by
> introducing independent set of quirks for each of the tcons.
>
> Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index cc6b05ca2c69..b7234eef3c7b 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -1508,6 +1508,8 @@ const struct of_device_id sun4i_tcon_of_table[] = {
>  	{ .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks },
>  	{ .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks },
>  	{ .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks },
> +	{ .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_quirks },
> +	{ .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_quirks },

It wasn't ordered alphabetically, I've fixed it up while applying.

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH 3/5] ARM: dts: sun7i: Add LVDS panel support on A20
  2020-02-19 18:08   ` [PATCH 3/5] ARM: dts: sun7i: Add LVDS panel support " Andrey Lebedev
@ 2020-02-20 17:23     ` Maxime Ripard
  0 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2020-02-20 17:23 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

On Wed, Feb 19, 2020 at 08:08:56PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <andrey@lebedev.lt>
>
> Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and
> provide sample LVDS panel, connected to tcon0.
>
> Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi | 30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 92b5be97085d..d50263c1ca9a 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -47,6 +47,7 @@
>  #include <dt-bindings/dma/sun4i-a10.h>
>  #include <dt-bindings/clock/sun7i-a20-ccu.h>
>  #include <dt-bindings/reset/sun4i-a10-ccu.h>
> +#include <dt-bindings/pinctrl/sun4i-a10.h>
>
>  / {
>  	interrupt-parent = <&gic>;
> @@ -404,11 +405,12 @@
>  		};
>
>  		tcon0: lcd-controller@1c0c000 {
> -			compatible = "allwinner,sun7i-a20-tcon";
> +			compatible = "allwinner,sun7i-a20-tcon0",
> +				     "allwinner,sun7i-a20-tcon";
>  			reg = <0x01c0c000 0x1000>;
>  			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
> -			resets = <&ccu RST_TCON0>;
> -			reset-names = "lcd";
> +			resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;
> +			reset-names = "lcd", "lvds";
>  			clocks = <&ccu CLK_AHB_LCD0>,
>  				 <&ccu CLK_TCON0_CH0>,
>  				 <&ccu CLK_TCON0_CH1>;
> @@ -444,6 +446,11 @@
>  					#size-cells = <0>;
>  					reg = <1>;
>
> +					tcon0_out_lvds: endpoint@0 {
> +						reg = <0>;
> +						allwinner,tcon-channel = <0>;
> +					};
> +

This isn't necessarily true. The endpoint would be the same for an RGB
panel for example. I've followed what we're doing elsewhere and
removed that endpoint entirely while applying, thanks!
Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH 4/5] dt-bindings: display: sun4i: New compatibles for A20 tcons
  2020-02-19 18:08   ` [PATCH 4/5] dt-bindings: display: sun4i: New compatibles for A20 tcons Andrey Lebedev
@ 2020-02-20 17:24     ` Maxime Ripard
  0 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2020-02-20 17:24 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

On Wed, Feb 19, 2020 at 08:08:57PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <andrey@lebedev.lt>
>
> Document new compatibles used to differentiate between timing
> controllers on A20 (sun7i)
>
> Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> ---
>  .../bindings/display/allwinner,sun4i-a10-tcon.yaml          | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> index 86ad617d2327..c0f6bb16fa34 100644
> --- a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> +++ b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> @@ -46,6 +46,12 @@ properties:
>            - allwinner,sun50i-h6-tcon-tv
>          - const: allwinner,sun8i-a83t-tcon-tv
>
> +      - items:
> +        - enum:
> +          - allwinner,sun7i-a20-tcon0
> +          - allwinner,sun7i-a20-tcon1
> +        - const: allwinner,sun7i-a20-tcon
> +
>    reg:
>      maxItems: 1

It wasn't ordered propertly, I've fixed it up while applying

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH 5/5] drm/sun4i: tcon: Support LVDS output on Allwinner A20
  2020-02-19 18:08   ` [PATCH 5/5] drm/sun4i: tcon: Support LVDS output on Allwinner A20 Andrey Lebedev
@ 2020-02-20 17:25     ` Maxime Ripard
  0 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2020-02-20 17:25 UTC (permalink / raw)
  To: Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, Andrey Lebedev,
	wens, daniel, linux-arm-kernel

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

On Wed, Feb 19, 2020 at 08:08:58PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <andrey@lebedev.lt>
>
> A20 SoC (found in Cubieboard 2 among others) requires different LVDS set
> up procedure than A33. Timing controller (tcon) driver only implements
> sun6i-style procedure, that doesn't work on A20 (sun7i).
>
> Signed-off-by: Andrey Lebedev <andrey@lebedev.lt>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 37 +++++++++++++++++++++++++++++-
>  drivers/gpu/drm/sun4i/sun4i_tcon.h | 11 +++++++++
>  2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index b7234eef3c7b..09ee6e8c6914 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -114,6 +114,30 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
>  	}
>  }
>
> +static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
> +				      const struct drm_encoder *encoder)
> +{
> +	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(2); /* delay at least 1200 ns */
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
> +			   SUN4I_TCON0_LVDS_ANA1_INIT,
> +			   SUN4I_TCON0_LVDS_ANA1_INIT);
> +	udelay(1); /* delay at least 120 ns */
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
> +			   SUN4I_TCON0_LVDS_ANA1_UPDATE,
> +			   SUN4I_TCON0_LVDS_ANA1_UPDATE);
> +	regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
> +			   SUN4I_TCON0_LVDS_ANA0_EN_MB,
> +			   SUN4I_TCON0_LVDS_ANA0_EN_MB);
> +}
> +
>  static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
>  				      const struct drm_encoder *encoder)
>  {
> @@ -1455,7 +1479,18 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
>  	.dclk_min_div		= 1,
>  };
>
> +static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = {
> +	.supports_lvds		= true,
> +	.has_channel_0		= true,
> +	.has_channel_1		= true,
> +	.dclk_min_div		= 4,
> +	/* Same display pipeline structure as A10 */
> +	.set_mux		= sun4i_a10_tcon_set_mux,
> +	.setup_lvds_phy		= sun4i_tcon_setup_lvds_phy,
> +};
> +
>  static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
> +	.supports_lvds		= false,

False is already the default here.

I've removed it while applying

Maxime

[-- 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] 36+ messages in thread

* Re: [PATCH 1/5] drm/sun4i: tcon: Introduce LVDS setup routine setting
  2020-02-20 17:21     ` Maxime Ripard
@ 2020-02-20 18:19       ` Andrey Lebedev
  0 siblings, 0 replies; 36+ messages in thread
From: Andrey Lebedev @ 2020-02-20 18:19 UTC (permalink / raw)
  To: Maxime Ripard, Andrey Lebedev
  Cc: airlied, linux-sunxi, linux-kernel, dri-devel, wens, daniel,
	linux-arm-kernel

On 2/20/20 7:21 PM, Maxime Ripard wrote:
>> +	regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
>> +			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),
>> +			  SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));
>> +
>> +}
>> +
> There's an extra blank line here that was reported by checkpatch. I've
> fixed it up while applying.

Weird, checkpatch didn't warn me about that:

./scripts/checkpatch.pl 
patches/0001-drm-sun4i-tcon-Introduce-LVDS-setup-routine-setting.patch
total: 0 errors, 0 warnings, 103 lines checked

patches/0001-drm-sun4i-tcon-Introduce-LVDS-setup-routine-setting.patch 
has no obvious style problems and is ready for submission.

In any case, thanks for correcting it!

-- 
Andrey Lebedev aka -.- . -.. -.. . .-.
Software engineer
Homepage: http://lebedev.lt/

_______________________________________________
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] 36+ messages in thread

end of thread, back to index

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10 19:56 [PATCH 1/1] Support LVDS output on Allwinner A20 Andrey Lebedev
2020-02-11  7:20 ` Maxime Ripard
2020-02-11 20:48   ` Andrey Lebedev
2020-02-12 12:53     ` Maxime Ripard
2020-02-12 22:46       ` Andrey Lebedev
2020-02-13  9:24         ` Maxime Ripard
2020-02-13 18:11           ` Andrey Lebedev
2020-02-14  7:58             ` Maxime Ripard
2020-02-12 22:23 ` [PATCH v2 1/2] ARM: sun7i: " andrey.lebedev
2020-02-13  9:32   ` Maxime Ripard
2020-02-12 22:23 ` [PATCH v2 2/2] ARM: sun7i: dts: Add LVDS panel support on A20 andrey.lebedev
2020-02-13  9:43   ` Maxime Ripard
2020-02-13 20:08     ` Andrey Lebedev
2020-02-14  7:52       ` Maxime Ripard
2020-02-14  8:43         ` Andrey Lebedev
2020-02-14  8:53           ` Maxime Ripard
2020-02-14 21:32             ` Andrey Lebedev
2020-02-17 17:51               ` Maxime Ripard
2020-02-18 17:50                 ` Andrey Lebedev
2020-02-19 12:06                   ` Maxime Ripard
2020-02-13 20:18 ` [PATCH v3 1/3] drm/sun4i: tcon: Introduce LVDS setup routine setting Andrey Lebedev
2020-02-13 20:18 ` [PATCH v3 2/3] drm/sun4i: tcon: Support LVDS output on Allwinner A20 Andrey Lebedev
2020-02-13 20:18 ` [PATCH v3 3/3] ARM: dts: sun7i: Add LVDS panel support on A20 Andrey Lebedev
2020-02-14  8:55   ` Maxime Ripard
2020-02-19 18:08 ` PATCH v4 Andrey Lebedev
2020-02-19 18:08   ` [PATCH 1/5] drm/sun4i: tcon: Introduce LVDS setup routine setting Andrey Lebedev
2020-02-20 17:21     ` Maxime Ripard
2020-02-20 18:19       ` Andrey Lebedev
2020-02-19 18:08   ` [PATCH 2/5] drm/sun4i: tcon: Separate quirks for tcon0 and tcon1 on A20 Andrey Lebedev
2020-02-20 17:22     ` Maxime Ripard
2020-02-19 18:08   ` [PATCH 3/5] ARM: dts: sun7i: Add LVDS panel support " Andrey Lebedev
2020-02-20 17:23     ` Maxime Ripard
2020-02-19 18:08   ` [PATCH 4/5] dt-bindings: display: sun4i: New compatibles for A20 tcons Andrey Lebedev
2020-02-20 17:24     ` Maxime Ripard
2020-02-19 18:08   ` [PATCH 5/5] drm/sun4i: tcon: Support LVDS output on Allwinner A20 Andrey Lebedev
2020-02-20 17:25     ` Maxime Ripard

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git