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
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
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/
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
From: Andrey Lebedev <[email protected]>
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 <[email protected]>
---
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
From: Andrey Lebedev <[email protected]>
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 <[email protected]>
---
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
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/
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
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, [email protected] wrote:
> From: Andrey Lebedev <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
On Thu, Feb 13, 2020 at 12:23:57AM +0200, [email protected] wrote:
> From: Andrey Lebedev <[email protected]>
>
> 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 <[email protected]>
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
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/
From: Andrey Lebedev <[email protected]>
Different sunxi flavors require slightly different sequence for enabling
LVDS output. This allows to differentiate between them.
Signed-off-by: Andrey Lebedev <[email protected]>
---
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
From: Andrey Lebedev <[email protected]>
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 <[email protected]>
---
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
From: Andrey Lebedev <[email protected]>
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 <[email protected]>
---
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
On Thu, Feb 13, 2020 at 10:43:04AM +0100, Maxime Ripard wrote:
> On Thu, Feb 13, 2020 at 12:23:57AM +0200, [email protected] wrote:
> > From: Andrey Lebedev <[email protected]>
> >
> > 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 <[email protected]>
>
> 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/
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, [email protected] wrote:
> > > From: Andrey Lebedev <[email protected]>
> > >
> > > 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 <[email protected]>
> >
> > 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
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
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/
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
On Thu, Feb 13, 2020 at 10:18:57PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
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 <[email protected]>
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 <[email protected]>
---
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/
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 <[email protected]>
> 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 <[email protected]>
> ---
> 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
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 <[email protected]>
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 <[email protected]>
---
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/
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 <[email protected]>
> 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 <[email protected]>
> ---
> 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
Address all outstanding review comments.
Maxime, please confirm I've got "document the new
compatibles" part right.
From: Andrey Lebedev <[email protected]>
Different sunxi flavors require slightly different sequence for enabling
LVDS output. This allows to differentiate between them.
Signed-off-by: Andrey Lebedev <[email protected]>
---
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
From: Andrey Lebedev <[email protected]>
Document new compatibles used to differentiate between timing
controllers on A20 (sun7i)
Signed-off-by: Andrey Lebedev <[email protected]>
---
.../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
From: Andrey Lebedev <[email protected]>
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 <[email protected]>
---
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
From: Andrey Lebedev <[email protected]>
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 <[email protected]>
---
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
From: Andrey Lebedev <[email protected]>
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 <[email protected]>
---
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
On Wed, Feb 19, 2020 at 08:08:54PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <[email protected]>
>
> Different sunxi flavors require slightly different sequence for enabling
> LVDS output. This allows to differentiate between them.
>
> Signed-off-by: Andrey Lebedev <[email protected]>
> ---
> 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
On Wed, Feb 19, 2020 at 08:08:56PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
On Wed, Feb 19, 2020 at 08:08:55PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
On Wed, Feb 19, 2020 at 08:08:57PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <[email protected]>
>
> Document new compatibles used to differentiate between timing
> controllers on A20 (sun7i)
>
> Signed-off-by: Andrey Lebedev <[email protected]>
> ---
> .../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
On Wed, Feb 19, 2020 at 08:08:58PM +0200, Andrey Lebedev wrote:
> From: Andrey Lebedev <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
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/
Hello Maxime,
Since Linus' merge window is now open, do I have to do anything to get
this merged into the mainline kernel?
On 2/20/20 7:25 PM, Maxime Ripard wrote:
> On Wed, Feb 19, 2020 at 08:08:58PM +0200, Andrey Lebedev wrote:
>> From: Andrey Lebedev <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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
>
--
Andrey Lebedev aka -.- . -.. -.. . .-.
Software engineer
Homepage: http://lebedev.lt/
Hi Andrey,
On Wed, Apr 01, 2020 at 01:59:27PM +0300, Andrey Lebedev wrote:
> Hello Maxime,
>
> Since Linus' merge window is now open, do I have to do anything to get this
> merged into the mainline kernel?
You don't have to do anything, it's already on its way to Linus:
https://lore.kernel.org/lkml/CAPM=9twza_DeycOEhT+u6Erh0yFTAUe447J6bxWCLq5+QW8ZaA@mail.gmail.com/
Maxime