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 */
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;elseval = 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));@@ -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/cc8c8bab2f2f2752ba6b11632dcd0f41bac249... [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....
+#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.
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/cc8c8bab2f2f2752ba6b11632dcd0f41bac249... [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....
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
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.
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
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.
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
From: Andrey Lebedev andrey@lebedev.lt
A20 SoC (found in Cubieboard 2 among others) requires different LVDS set up procedure than A33. Timing controller (tcon) driver only implements sun6i-style procedure, that doesn't work on A20 (sun7i).
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 95 ++++++++++++++++++++---------- drivers/gpu/drm/sun4i/sun4i_tcon.h | 14 +++++ 2 files changed, 77 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index c81cdce6ed55..e4c605ca685e 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -114,46 +114,73 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel, } }
+static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon, + const struct drm_encoder *encoder) +{ + u8 val; + + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_C(2) | + SUN6I_TCON0_LVDS_ANA0_V(3) | + SUN6I_TCON0_LVDS_ANA0_PD(2) | + SUN6I_TCON0_LVDS_ANA0_EN_LDO); + udelay(2); + + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_EN_MB, + SUN6I_TCON0_LVDS_ANA0_EN_MB); + udelay(2); + + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_EN_DRVC, + SUN6I_TCON0_LVDS_ANA0_EN_DRVC); + + if (sun4i_tcon_get_pixel_depth(encoder) == 18) + val = 7; + else + val = 0xf; + + regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf), + SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val)); + +} + +static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon, + const struct drm_encoder *encoder) +{ + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN4I_TCON0_LVDS_ANA0_CK_EN | + SUN4I_TCON0_LVDS_ANA0_REG_V | + SUN4I_TCON0_LVDS_ANA0_REG_C | + SUN4I_TCON0_LVDS_ANA0_EN_MB | + SUN4I_TCON0_LVDS_ANA0_PD | + SUN4I_TCON0_LVDS_ANA0_DCHS); + + udelay(2); /* delay at least 1200 ns */ + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, + SUN4I_TCON0_LVDS_ANA1_INIT, + SUN4I_TCON0_LVDS_ANA1_INIT); + udelay(1); /* delay at least 120 ns */ + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, + SUN4I_TCON0_LVDS_ANA1_UPDATE, + SUN4I_TCON0_LVDS_ANA1_UPDATE); + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN4I_TCON0_LVDS_ANA0_EN_MB, + SUN4I_TCON0_LVDS_ANA0_EN_MB); +} + + static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon, const struct drm_encoder *encoder, bool enabled) { if (enabled) { - u8 val; - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG, SUN4I_TCON0_LVDS_IF_EN, SUN4I_TCON0_LVDS_IF_EN); - - /* - * As their name suggest, these values only apply to the A31 - * and later SoCs. We'll have to rework this when merging - * support for the older SoCs. - */ - regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_C(2) | - SUN6I_TCON0_LVDS_ANA0_V(3) | - SUN6I_TCON0_LVDS_ANA0_PD(2) | - SUN6I_TCON0_LVDS_ANA0_EN_LDO); - udelay(2); - - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_EN_MB, - SUN6I_TCON0_LVDS_ANA0_EN_MB); - udelay(2); - - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_EN_DRVC, - SUN6I_TCON0_LVDS_ANA0_EN_DRVC); - - if (sun4i_tcon_get_pixel_depth(encoder) == 18) - val = 7; - else - val = 0xf; - - regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf), - SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val)); + if (tcon->quirks->setup_lvds_phy) + tcon->quirks->setup_lvds_phy(tcon, encoder); } else { regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG, SUN4I_TCON0_LVDS_IF_EN, 0); @@ -1454,23 +1481,27 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { };
static const struct sun4i_tcon_quirks sun7i_a20_quirks = { + .supports_lvds = true, .has_channel_0 = true, .has_channel_1 = true, .dclk_min_div = 4, /* Same display pipeline structure as A10 */ .set_mux = sun4i_a10_tcon_set_mux, + .setup_lvds_phy = sun4i_tcon_setup_lvds_phy, };
static const struct sun4i_tcon_quirks sun8i_a33_quirks = { .has_channel_0 = true, .has_lvds_alt = true, .dclk_min_div = 1, + .setup_lvds_phy = sun6i_tcon_setup_lvds_phy, };
static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = { .supports_lvds = true, .has_channel_0 = true, .dclk_min_div = 1, + .setup_lvds_phy = sun6i_tcon_setup_lvds_phy, };
static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = { diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index a62ec826ae71..cfbf4e6c1679 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -193,6 +193,13 @@ #define SUN4I_TCON_MUX_CTRL_REG 0x200
#define SUN4I_TCON0_LVDS_ANA0_REG 0x220 +#define SUN4I_TCON0_LVDS_ANA0_DCHS BIT(16) +#define SUN4I_TCON0_LVDS_ANA0_PD (BIT(20) | BIT(21)) +#define SUN4I_TCON0_LVDS_ANA0_EN_MB BIT(22) +#define SUN4I_TCON0_LVDS_ANA0_REG_C (BIT(24) | BIT(25)) +#define SUN4I_TCON0_LVDS_ANA0_REG_V (BIT(26) | BIT(27)) +#define SUN4I_TCON0_LVDS_ANA0_CK_EN (BIT(29) | BIT(28)) + #define SUN6I_TCON0_LVDS_ANA0_EN_MB BIT(31) #define SUN6I_TCON0_LVDS_ANA0_EN_LDO BIT(30) #define SUN6I_TCON0_LVDS_ANA0_EN_DRVC BIT(24) @@ -201,6 +208,10 @@ #define SUN6I_TCON0_LVDS_ANA0_V(x) (((x) & 3) << 8) #define SUN6I_TCON0_LVDS_ANA0_PD(x) (((x) & 3) << 4)
+#define SUN4I_TCON0_LVDS_ANA1_REG 0x224 +#define SUN4I_TCON0_LVDS_ANA1_INIT (0x1f << 26 | 0x1f << 10) +#define SUN4I_TCON0_LVDS_ANA1_UPDATE (0x1f << 16 | 0x1f << 00) + #define SUN4I_TCON1_FILL_CTL_REG 0x300 #define SUN4I_TCON1_FILL_BEG0_REG 0x304 #define SUN4I_TCON1_FILL_END0_REG 0x308 @@ -228,6 +239,9 @@ struct sun4i_tcon_quirks {
/* callback to handle tcon muxing options */ int (*set_mux)(struct sun4i_tcon *, const struct drm_encoder *); + /* handler for LVDS setup routine */ + void (*setup_lvds_phy)(struct sun4i_tcon *tcon, + const struct drm_encoder *encoder); };
struct sun4i_tcon {
Hi,
The prefix of your title should be "drm/sun4i: tcon:" instead of "ARM: sun7i:"
The latter would be if you were modifying files under arch/arm, but you're modifying files in (drivers/gpu/)drm/sun4i :)
On Thu, Feb 13, 2020 at 12:23:55AM +0200, andrey.lebedev@gmail.com wrote:
From: Andrey Lebedev andrey@lebedev.lt
A20 SoC (found in Cubieboard 2 among others) requires different LVDS set up procedure than A33. Timing controller (tcon) driver only implements sun6i-style procedure, that doesn't work on A20 (sun7i).
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
drivers/gpu/drm/sun4i/sun4i_tcon.c | 95 ++++++++++++++++++++---------- drivers/gpu/drm/sun4i/sun4i_tcon.h | 14 +++++ 2 files changed, 77 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index c81cdce6ed55..e4c605ca685e 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -114,46 +114,73 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel, } }
+static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
const struct drm_encoder *encoder)+{
- u8 val;
- regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN6I_TCON0_LVDS_ANA0_C(2) |SUN6I_TCON0_LVDS_ANA0_V(3) |SUN6I_TCON0_LVDS_ANA0_PD(2) |SUN6I_TCON0_LVDS_ANA0_EN_LDO);- udelay(2);
- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN6I_TCON0_LVDS_ANA0_EN_MB,SUN6I_TCON0_LVDS_ANA0_EN_MB);- udelay(2);
- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN6I_TCON0_LVDS_ANA0_EN_DRVC,SUN6I_TCON0_LVDS_ANA0_EN_DRVC);- if (sun4i_tcon_get_pixel_depth(encoder) == 18)
val = 7;- else
val = 0xf;- regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));+}
+static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
const struct drm_encoder *encoder)+{
- regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN4I_TCON0_LVDS_ANA0_CK_EN |SUN4I_TCON0_LVDS_ANA0_REG_V |SUN4I_TCON0_LVDS_ANA0_REG_C |SUN4I_TCON0_LVDS_ANA0_EN_MB |SUN4I_TCON0_LVDS_ANA0_PD |SUN4I_TCON0_LVDS_ANA0_DCHS);- udelay(2); /* delay at least 1200 ns */
- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
SUN4I_TCON0_LVDS_ANA1_INIT,SUN4I_TCON0_LVDS_ANA1_INIT);- udelay(1); /* delay at least 120 ns */
- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
SUN4I_TCON0_LVDS_ANA1_UPDATE,SUN4I_TCON0_LVDS_ANA1_UPDATE);- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN4I_TCON0_LVDS_ANA0_EN_MB,SUN4I_TCON0_LVDS_ANA0_EN_MB);+}
Patches should contain only one logical change, so ideally this should be two patches: one to create the function pointer and fill it for the A31 style setup, the other one to add support for the A20.
Also, you should have only a single line between the two functions and the second should come before the first (alphabetical ordering).
Maxime
From: Andrey Lebedev andrey@lebedev.lt
Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and provide sample LVDS panel, connected to tcon0.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- arch/arm/boot/dts/sun7i-a20.dtsi | 45 +++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 92b5be97085d..b05fdf8df32e 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -47,6 +47,7 @@ #include <dt-bindings/dma/sun4i-a10.h> #include <dt-bindings/clock/sun7i-a20-ccu.h> #include <dt-bindings/reset/sun4i-a10-ccu.h> +#include <dt-bindings/pinctrl/sun4i-a10.h>
/ { interrupt-parent = <&gic>; @@ -407,8 +408,8 @@ compatible = "allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; - resets = <&ccu RST_TCON0>; - reset-names = "lcd"; + resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; + reset-names = "lcd", "lvds"; clocks = <&ccu CLK_AHB_LCD0>, <&ccu CLK_TCON0_CH0>, <&ccu CLK_TCON0_CH1>; @@ -444,6 +445,11 @@ #size-cells = <0>; reg = <1>;
+ tcon0_out_lvds: endpoint@0 { + reg = <0>; + remote-endpoint = <&lvds_in_tcon0>; + allwinner,tcon-channel = <0>; + }; tcon0_out_hdmi: endpoint@1 { reg = <1>; remote-endpoint = <&hdmi_in_tcon0>; @@ -686,6 +692,19 @@ }; };
+ lvds_panel: panel@1c16500 { + compatible = "panel-lvds"; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + + port { + lvds_in_tcon0: endpoint { + remote-endpoint = <&tcon0_out_lvds>; + }; + }; + }; + spi2: spi@1c17000 { compatible = "allwinner,sun4i-a10-spi"; reg = <0x01c17000 0x1000>; @@ -872,7 +891,7 @@ gmac_rgmii_pins: gmac-rgmii-pins { pins = "PA0", "PA1", "PA2", "PA3", "PA4", "PA5", "PA6", - "PA7", "PA8", "PA10", + "PA7", "PA8", "PA10", "PA11", "PA12", "PA13", "PA15", "PA16"; function = "gmac"; @@ -1162,6 +1181,26 @@ pins = "PI20", "PI21"; function = "uart7"; }; + + /omit-if-no-ref/ + lcd_lvds0_pins: lcd_lvds0_pins { + allwinner,pins = + "PD0", "PD1", "PD2", "PD3", "PD4", + "PD5", "PD6", "PD7", "PD8", "PD9"; + allwinner,function = "lvds0"; + allwinner,drive = <SUN4I_PINCTRL_10_MA>; + allwinner,pull = <SUN4I_PINCTRL_NO_PULL>; + }; + + /omit-if-no-ref/ + lcd_lvds1_pins: lcd_lvds1_pins { + allwinner,pins = + "PD10", "PD11", "PD12", "PD13", "PD14", + "PD15", "PD16", "PD17", "PD18", "PD19"; + allwinner,function = "lvds1"; + allwinner,drive = <SUN4I_PINCTRL_10_MA>; + allwinner,pull = <SUN4I_PINCTRL_NO_PULL>; + }; };
timer@1c20c00 {
On Thu, Feb 13, 2020 at 12:23:57AM +0200, andrey.lebedev@gmail.com wrote:
From: Andrey Lebedev andrey@lebedev.lt
Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and provide sample LVDS panel, connected to tcon0.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
And this prefix should be ARM: dts: sun7i ;)
arch/arm/boot/dts/sun7i-a20.dtsi | 45 +++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 92b5be97085d..b05fdf8df32e 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -47,6 +47,7 @@ #include <dt-bindings/dma/sun4i-a10.h> #include <dt-bindings/clock/sun7i-a20-ccu.h> #include <dt-bindings/reset/sun4i-a10-ccu.h> +#include <dt-bindings/pinctrl/sun4i-a10.h>
/ { interrupt-parent = <&gic>; @@ -407,8 +408,8 @@ compatible = "allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
resets = <&ccu RST_TCON0>;reset-names = "lcd";
resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;reset-names = "lcd", "lvds"; clocks = <&ccu CLK_AHB_LCD0>, <&ccu CLK_TCON0_CH0>, <&ccu CLK_TCON0_CH1>;@@ -444,6 +445,11 @@ #size-cells = <0>; reg = <1>;
tcon0_out_lvds: endpoint@0 {reg = <0>;remote-endpoint = <&lvds_in_tcon0>;allwinner,tcon-channel = <0>;};
A new line here would be nice
tcon0_out_hdmi: endpoint@1 { reg = <1>; remote-endpoint = <&hdmi_in_tcon0>;@@ -686,6 +692,19 @@ }; };
lvds_panel: panel@1c16500 {compatible = "panel-lvds";#address-cells = <1>;#size-cells = <0>;status = "disabled";port {lvds_in_tcon0: endpoint {remote-endpoint = <&tcon0_out_lvds>;};};};
There's no point in creating that panel.
spi2: spi@1c17000 { compatible = "allwinner,sun4i-a10-spi"; reg = <0x01c17000 0x1000>;@@ -872,7 +891,7 @@ gmac_rgmii_pins: gmac-rgmii-pins { pins = "PA0", "PA1", "PA2", "PA3", "PA4", "PA5", "PA6",
"PA7", "PA8", "PA10",
"PA7", "PA8", "PA10", "PA11", "PA12", "PA13", "PA15", "PA16"; function = "gmac";@@ -1162,6 +1181,26 @@ pins = "PI20", "PI21"; function = "uart7"; };
/omit-if-no-ref/lcd_lvds0_pins: lcd_lvds0_pins {
underscores in the node names will create a dtc warning at compilation, you should use lcd-lvds0-pins instead.
allwinner,pins ="PD0", "PD1", "PD2", "PD3", "PD4","PD5", "PD6", "PD7", "PD8", "PD9";allwinner,function = "lvds0";allwinner,drive = <SUN4I_PINCTRL_10_MA>;allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
Those properties are deprecated and should be replaced by pins and functions. allwinner,drive and allwinner,pull are at their default values and can be dropped.
This will create a spurious warning message for TCON1, since we adjusted the driver to tell it supports LVDS, but there's no LVDS reset line, so we need to make it finer grained.
Maybe adding a tcon0 / tcon1 compatible? Chen-Yu, any thought?
Maxime
On Thu, Feb 13, 2020 at 10:43:04AM +0100, Maxime Ripard wrote:
On Thu, Feb 13, 2020 at 12:23:57AM +0200, andrey.lebedev@gmail.com wrote:
From: Andrey Lebedev andrey@lebedev.lt
Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and provide sample LVDS panel, connected to tcon0.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
And this prefix should be ARM: dts: sun7i ;)
Ah, thanks, I think I've got the pattern now!
/omit-if-no-ref/lcd_lvds0_pins: lcd_lvds0_pins {underscores in the node names will create a dtc warning at compilation, you should use lcd-lvds0-pins instead.
You're right, I wasn't following the naming convention here. dtc doesn't produce any warning on this though. Fixed that anyway.
This will create a spurious warning message for TCON1, since we adjusted the driver to tell it supports LVDS, but there's no LVDS reset line, so we need to make it finer grained.
Yes, I can attribute two of the messages in my dmesg log [1] to this ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they can be confusing to someone.
I'd need some pointers on how to deal with that though (if we want to do it in this scope).
[1] excerpt from kernel boot log:
[ 4.890975] sun4i-drm display-engine: bound 1e00000.display-frontend (ops sun4i_frontend_driver_exit [sun4i_frontend]) [ 4.902032] sun4i-drm display-engine: bound 1e20000.display-frontend (ops sun4i_frontend_driver_exit [sun4i_frontend]) [ 4.913467] sun4i-drm display-engine: bound 1e60000.display-backend (ops sun4i_backend_ops [sun4i_backend]) [ 4.923806] sun4i-drm display-engine: bound 1e40000.display-backend (ops sun4i_backend_ops [sun4i_backend]) [ 4.934451] sun4i-drm display-engine: bound 1c0c000.lcd-controller (ops sun4i_tcon_platform_driver_exit [sun4i_tcon]) [ 4.945254] sun4i-tcon 1c0d000.lcd-controller: Missing LVDS properties, Please upgrade your DT [ 4.953935] sun4i-tcon 1c0d000.lcd-controller: LVDS output disabled [ 4.960857] sun4i-drm display-engine: No panel or bridge found... RGB output disabled [ 4.968845] sun4i-drm display-engine: bound 1c0d000.lcd-controller (ops sun4i_tcon_platform_driver_exit [sun4i_tcon]) [ 5.080874] sun4i-drm display-engine: bound 1c16000.hdmi (ops sun4i_hdmi_driver_exit [sun4i_drm_hdmi]) [ 5.110087] [drm] Initialized sun4i-drm 1.0.0 20150629 for display-engine on minor 0 [ 5.763064] sun4i-drm display-engine: fb0: sun4i-drmdrmfb frame buffer device
Hi,
On Thu, Feb 13, 2020 at 10:08:23PM +0200, Andrey Lebedev wrote:
On Thu, Feb 13, 2020 at 10:43:04AM +0100, Maxime Ripard wrote:
On Thu, Feb 13, 2020 at 12:23:57AM +0200, andrey.lebedev@gmail.com wrote:
From: Andrey Lebedev andrey@lebedev.lt
Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and provide sample LVDS panel, connected to tcon0.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
And this prefix should be ARM: dts: sun7i ;)
Ah, thanks, I think I've got the pattern now!
/omit-if-no-ref/lcd_lvds0_pins: lcd_lvds0_pins {underscores in the node names will create a dtc warning at compilation, you should use lcd-lvds0-pins instead.
It's silenced by default, but you'll get them with W=1
You're right, I wasn't following the naming convention here. dtc doesn't produce any warning on this though. Fixed that anyway.
This will create a spurious warning message for TCON1, since we adjusted the driver to tell it supports LVDS, but there's no LVDS reset line, so we need to make it finer grained.
Yes, I can attribute two of the messages in my dmesg log [1] to this ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they can be confusing to someone.
I'd need some pointers on how to deal with that though (if we want to do it in this scope).
Like I was mentionning, you could introduce a new compatible for each TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0
Maxime
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?
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 Fri, Feb 14, 2020 at 09:53:51AM +0100, Maxime Ripard wrote:
On Fri, Feb 14, 2020 at 10:43:58AM +0200, Andrey Lebedev wrote:
On Fri, Feb 14, 2020 at 08:52:18AM +0100, Maxime Ripard wrote:
This will create a spurious warning message for TCON1, since we adjusted the driver to tell it supports LVDS, but there's no LVDS reset line, so we need to make it finer grained.
Yes, I can attribute two of the messages in my dmesg log [1] to this ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they can be confusing to someone.
I'd need some pointers on how to deal with that though (if we want to do it in this scope).
Like I was mentionning, you could introduce a new compatible for each TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0
Can you give me an idea how that compatible might look like?
tcon0: lcd-controller@1c0c000 { compatible = "allwinner,sun7i-a20-tcon", "allwinner,lvds";or
tcon0: lcd-controller@1c0c000 { compatible = "allwinner,sun7i-a20-tcon", "allwinner,tcon0";? Or something completely different?
Something like
&tcon0 { compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon"; };
&tcon1 { compatible = "allwinner,sun7i-a20-tcon1", "allwinner,sun7i-a20-tcon"; };
Hi Maxime, here is what I came up with, please take a look. If the approach is right, I'll split it up and include into the patch set.
From f3e45c958a9551a52ac26435785bdb572e54d8db Mon Sep 17 00:00:00 2001
From: Andrey Lebedev andrey@lebedev.lt Date: Fri, 14 Feb 2020 23:21:59 +0200 Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20
This allows to avoid warnings about reset line not provided for tcon1.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- arch/arm/boot/dts/sun7i-a20.dtsi | 2 +- drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 +++++++++++++++++++++- drivers/gpu/drm/sun4i/sun4i_tcon.h | 2 ++ 3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 3b3c366a2bee..bab59fc4d9b1 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -405,7 +405,7 @@ };
tcon0: lcd-controller@1c0c000 { - compatible = "allwinner,sun7i-a20-tcon"; + compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 800a9bd86112..cb2040aec436 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1107,6 +1107,25 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv, return sun4i_tcon_find_engine_traverse(drv, node, 0); }
+/* + * Check if given tcon supports LVDS + * + * Some of the sunxi SoC variants contain several timing controllers, but only + * one of them can be used to drive LVDS screen. In this case such tcon is + * identified in respective quirks struct: lvds_compatible_tcon property will + * hold "compatible" string of the tcon, that supports LVDS. + * + * If lvds_compatible_tcon is not set, all tcons are considered capable of + * driving LVDS. + */ +static bool sun4i_tcon_lvds_compat(struct device *dev, struct sun4i_tcon *tcon) +{ + if (tcon->quirks->lvds_compatible_tcon == NULL) + return true; + return of_device_is_compatible(dev->of_node, + tcon->quirks->lvds_compatible_tcon); +} + static int sun4i_tcon_bind(struct device *dev, struct device *master, void *data) { @@ -1161,7 +1180,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, return ret; }
- if (tcon->quirks->supports_lvds) { + if (tcon->quirks->supports_lvds && sun4i_tcon_lvds_compat(dev, tcon)) { /* * This can only be made optional since we've had DT * nodes without the LVDS reset properties. @@ -1481,6 +1500,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
static const struct sun4i_tcon_quirks sun7i_a20_quirks = { .supports_lvds = true, + .lvds_compatible_tcon = "allwinner,sun7i-a20-tcon0", .has_channel_0 = true, .has_channel_1 = true, .dclk_min_div = 4, diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index cfbf4e6c1679..bc87d28ee341 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks { bool needs_de_be_mux; /* sun6i needs mux to select backend */ bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */ bool supports_lvds; /* Does the TCON support an LVDS output? */ + /* "compatible" string of TCON that exclusively supports LVDS */ + const char *lvds_compatible_tcon; u8 dclk_min_div; /* minimum divider for TCON0 DCLK */
/* callback to handle tcon muxing options */
Hi Andrey,
On Fri, Feb 14, 2020 at 11:32:31PM +0200, Andrey Lebedev wrote:
On Fri, Feb 14, 2020 at 09:53:51AM +0100, Maxime Ripard wrote:
On Fri, Feb 14, 2020 at 10:43:58AM +0200, Andrey Lebedev wrote:
On Fri, Feb 14, 2020 at 08:52:18AM +0100, Maxime Ripard wrote:
This will create a spurious warning message for TCON1, since we adjusted the driver to tell it supports LVDS, but there's no LVDS reset line, so we need to make it finer grained.
Yes, I can attribute two of the messages in my dmesg log [1] to this ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they can be confusing to someone.
I'd need some pointers on how to deal with that though (if we want to do it in this scope).
Like I was mentionning, you could introduce a new compatible for each TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0
Can you give me an idea how that compatible might look like?
tcon0: lcd-controller@1c0c000 { compatible = "allwinner,sun7i-a20-tcon", "allwinner,lvds";or
tcon0: lcd-controller@1c0c000 { compatible = "allwinner,sun7i-a20-tcon", "allwinner,tcon0";? Or something completely different?
Something like
&tcon0 { compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon"; };
&tcon1 { compatible = "allwinner,sun7i-a20-tcon1", "allwinner,sun7i-a20-tcon"; };
Hi Maxime, here is what I came up with, please take a look. If the approach is right, I'll split it up and include into the patch set.
From f3e45c958a9551a52ac26435785bdb572e54d8db Mon Sep 17 00:00:00 2001 From: Andrey Lebedev andrey@lebedev.lt Date: Fri, 14 Feb 2020 23:21:59 +0200 Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20
This allows to avoid warnings about reset line not provided for tcon1.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
arch/arm/boot/dts/sun7i-a20.dtsi | 2 +- drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 +++++++++++++++++++++- drivers/gpu/drm/sun4i/sun4i_tcon.h | 2 ++ 3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 3b3c366a2bee..bab59fc4d9b1 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -405,7 +405,7 @@ };
tcon0: lcd-controller@1c0c000 {
compatible = "allwinner,sun7i-a20-tcon";
compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon";
That's correct
reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 800a9bd86112..cb2040aec436 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1107,6 +1107,25 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv, return sun4i_tcon_find_engine_traverse(drv, node, 0); }
+/*
- Check if given tcon supports LVDS
- Some of the sunxi SoC variants contain several timing controllers, but only
- one of them can be used to drive LVDS screen. In this case such tcon is
- identified in respective quirks struct: lvds_compatible_tcon property will
- hold "compatible" string of the tcon, that supports LVDS.
- If lvds_compatible_tcon is not set, all tcons are considered capable of
- driving LVDS.
- */
+static bool sun4i_tcon_lvds_compat(struct device *dev, struct sun4i_tcon *tcon) +{
- if (tcon->quirks->lvds_compatible_tcon == NULL)
return true;- return of_device_is_compatible(dev->of_node,
tcon->quirks->lvds_compatible_tcon);+}
static int sun4i_tcon_bind(struct device *dev, struct device *master, void *data) { @@ -1161,7 +1180,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, return ret; }
- if (tcon->quirks->supports_lvds) {
- if (tcon->quirks->supports_lvds && sun4i_tcon_lvds_compat(dev, tcon)) { /*
- This can only be made optional since we've had DT
- nodes without the LVDS reset properties.
@@ -1481,6 +1500,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
static const struct sun4i_tcon_quirks sun7i_a20_quirks = { .supports_lvds = true,
- .lvds_compatible_tcon = "allwinner,sun7i-a20-tcon0", .has_channel_0 = true, .has_channel_1 = true, .dclk_min_div = 4,
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index cfbf4e6c1679..bc87d28ee341 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks { bool needs_de_be_mux; /* sun6i needs mux to select backend */ bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */ bool supports_lvds; /* Does the TCON support an LVDS output? */
- /* "compatible" string of TCON that exclusively supports LVDS */
- const char *lvds_compatible_tcon;
However this is far more complicated than needed, you can simply add a new quirks structure associated to the tcon0 compatible in sun4i_tcon_of_table, and that will do
Maxime
On Mon, Feb 17, 2020 at 06:51:35PM +0100, Maxime Ripard wrote:
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index cfbf4e6c1679..bc87d28ee341 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks { bool needs_de_be_mux; /* sun6i needs mux to select backend */ bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */ bool supports_lvds; /* Does the TCON support an LVDS output? */
- /* "compatible" string of TCON that exclusively supports LVDS */
- const char *lvds_compatible_tcon;
However this is far more complicated than needed, you can simply add a new quirks structure associated to the tcon0 compatible in sun4i_tcon_of_table, and that will do
Aha! Does this look good to you?
From 4ad2978e404c63d4cca1b7890bc5bdd4d1e8965d Mon Sep 17 00:00:00 2001
From: Andrey Lebedev andrey@lebedev.lt Date: Tue, 18 Feb 2020 19:47:33 +0200 Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20
This allows to avoid warnings about reset line not provided for tcon1.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- arch/arm/boot/dts/sun7i-a20.dtsi | 6 ++++-- drivers/gpu/drm/sun4i/sun4i_tcon.c | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index dc4f3f249aee..d50263c1ca9a 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -405,7 +405,8 @@ };
tcon0: lcd-controller@1c0c000 { - compatible = "allwinner,sun7i-a20-tcon"; + compatible = "allwinner,sun7i-a20-tcon0", + "allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; @@ -460,7 +461,8 @@ };
tcon1: lcd-controller@1c0d000 { - compatible = "allwinner,sun7i-a20-tcon"; + compatible = "allwinner,sun7i-a20-tcon1", + "allwinner,sun7i-a20-tcon"; reg = <0x01c0d000 0x1000>; interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>; resets = <&ccu RST_TCON1>; diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 800a9bd86112..d9605d331010 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1479,7 +1479,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { .dclk_min_div = 1, };
-static const struct sun4i_tcon_quirks sun7i_a20_quirks = { +static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = { .supports_lvds = true, .has_channel_0 = true, .has_channel_1 = true, @@ -1489,6 +1489,15 @@ static const struct sun4i_tcon_quirks sun7i_a20_quirks = { .setup_lvds_phy = sun4i_tcon_setup_lvds_phy, };
+static const struct sun4i_tcon_quirks sun7i_a20_tcon1_quirks = { + .supports_lvds = false, + .has_channel_0 = true, + .has_channel_1 = true, + .dclk_min_div = 4, + /* Same display pipeline structure as A10 */ + .set_mux = sun4i_a10_tcon_set_mux, +}; + static const struct sun4i_tcon_quirks sun8i_a33_quirks = { .has_channel_0 = true, .has_lvds_alt = true, @@ -1534,7 +1543,8 @@ const struct of_device_id sun4i_tcon_of_table[] = { { .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks }, { .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks }, { .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks }, - { .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks }, + { .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_tcon0_quirks }, + { .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_tcon1_quirks }, { .compatible = "allwinner,sun8i-a23-tcon", .data = &sun8i_a33_quirks }, { .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks }, { .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data = &sun8i_a83t_lcd_quirks },
On Tue, Feb 18, 2020 at 07:50:33PM +0200, Andrey Lebedev wrote:
On Mon, Feb 17, 2020 at 06:51:35PM +0100, Maxime Ripard wrote:
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index cfbf4e6c1679..bc87d28ee341 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks { bool needs_de_be_mux; /* sun6i needs mux to select backend */ bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */ bool supports_lvds; /* Does the TCON support an LVDS output? */
- /* "compatible" string of TCON that exclusively supports LVDS */
- const char *lvds_compatible_tcon;
However this is far more complicated than needed, you can simply add a new quirks structure associated to the tcon0 compatible in sun4i_tcon_of_table, and that will do
Aha! Does this look good to you?
From 4ad2978e404c63d4cca1b7890bc5bdd4d1e8965d Mon Sep 17 00:00:00 2001 From: Andrey Lebedev andrey@lebedev.lt Date: Tue, 18 Feb 2020 19:47:33 +0200 Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20
This allows to avoid warnings about reset line not provided for tcon1.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
arch/arm/boot/dts/sun7i-a20.dtsi | 6 ++++-- drivers/gpu/drm/sun4i/sun4i_tcon.c | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index dc4f3f249aee..d50263c1ca9a 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -405,7 +405,8 @@ };
tcon0: lcd-controller@1c0c000 {
compatible = "allwinner,sun7i-a20-tcon";
compatible = "allwinner,sun7i-a20-tcon0","allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;@@ -460,7 +461,8 @@ };
tcon1: lcd-controller@1c0d000 {
compatible = "allwinner,sun7i-a20-tcon";
compatible = "allwinner,sun7i-a20-tcon1","allwinner,sun7i-a20-tcon"; reg = <0x01c0d000 0x1000>; interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>; resets = <&ccu RST_TCON1>;
This needs to be in a separate patch
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 800a9bd86112..d9605d331010 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1479,7 +1479,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { .dclk_min_div = 1, };
-static const struct sun4i_tcon_quirks sun7i_a20_quirks = { +static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = { .supports_lvds = true, .has_channel_0 = true, .has_channel_1 = true, @@ -1489,6 +1489,15 @@ static const struct sun4i_tcon_quirks sun7i_a20_quirks = { .setup_lvds_phy = sun4i_tcon_setup_lvds_phy, };
+static const struct sun4i_tcon_quirks sun7i_a20_tcon1_quirks = {
- .supports_lvds = false,
- .has_channel_0 = true,
- .has_channel_1 = true,
- .dclk_min_div = 4,
- /* Same display pipeline structure as A10 */
- .set_mux = sun4i_a10_tcon_set_mux,
+};
static const struct sun4i_tcon_quirks sun8i_a33_quirks = { .has_channel_0 = true, .has_lvds_alt = true, @@ -1534,7 +1543,8 @@ const struct of_device_id sun4i_tcon_of_table[] = { { .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks }, { .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks }, { .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks },
- { .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks },
- { .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_tcon0_quirks },
- { .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_tcon1_quirks }, { .compatible = "allwinner,sun8i-a23-tcon", .data = &sun8i_a33_quirks }, { .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks }, { .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data = &sun8i_a83t_lcd_quirks },
And we need to support older DT as well, so you shouldn't remove the older TCON compatible and its structure. Make sure to document the new compatibles too.
Maxime
From: Andrey Lebedev andrey@lebedev.lt
Different sunxi flavors require slightly different sequence for enabling LVDS output. This allows to differentiate between them.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 68 ++++++++++++++++-------------- drivers/gpu/drm/sun4i/sun4i_tcon.h | 3 ++ 2 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index c81cdce6ed55..cc6b05ca2c69 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -114,46 +114,48 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel, } }
+static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon, + const struct drm_encoder *encoder) +{ + u8 val; + + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_C(2) | + SUN6I_TCON0_LVDS_ANA0_V(3) | + SUN6I_TCON0_LVDS_ANA0_PD(2) | + SUN6I_TCON0_LVDS_ANA0_EN_LDO); + udelay(2); + + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_EN_MB, + SUN6I_TCON0_LVDS_ANA0_EN_MB); + udelay(2); + + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_EN_DRVC, + SUN6I_TCON0_LVDS_ANA0_EN_DRVC); + + if (sun4i_tcon_get_pixel_depth(encoder) == 18) + val = 7; + else + val = 0xf; + + regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf), + SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val)); + +} + static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon, const struct drm_encoder *encoder, bool enabled) { if (enabled) { - u8 val; - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG, SUN4I_TCON0_LVDS_IF_EN, SUN4I_TCON0_LVDS_IF_EN); - - /* - * As their name suggest, these values only apply to the A31 - * and later SoCs. We'll have to rework this when merging - * support for the older SoCs. - */ - regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_C(2) | - SUN6I_TCON0_LVDS_ANA0_V(3) | - SUN6I_TCON0_LVDS_ANA0_PD(2) | - SUN6I_TCON0_LVDS_ANA0_EN_LDO); - udelay(2); - - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_EN_MB, - SUN6I_TCON0_LVDS_ANA0_EN_MB); - udelay(2); - - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_EN_DRVC, - SUN6I_TCON0_LVDS_ANA0_EN_DRVC); - - if (sun4i_tcon_get_pixel_depth(encoder) == 18) - val = 7; - else - val = 0xf; - - regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf), - SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val)); + if (tcon->quirks->setup_lvds_phy) + tcon->quirks->setup_lvds_phy(tcon, encoder); } else { regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG, SUN4I_TCON0_LVDS_IF_EN, 0); @@ -1465,12 +1467,14 @@ static const struct sun4i_tcon_quirks sun8i_a33_quirks = { .has_channel_0 = true, .has_lvds_alt = true, .dclk_min_div = 1, + .setup_lvds_phy = sun6i_tcon_setup_lvds_phy, };
static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = { .supports_lvds = true, .has_channel_0 = true, .dclk_min_div = 1, + .setup_lvds_phy = sun6i_tcon_setup_lvds_phy, };
static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = { diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index a62ec826ae71..2974e59ef9f2 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -228,6 +228,9 @@ struct sun4i_tcon_quirks {
/* callback to handle tcon muxing options */ int (*set_mux)(struct sun4i_tcon *, const struct drm_encoder *); + /* handler for LVDS setup routine */ + void (*setup_lvds_phy)(struct sun4i_tcon *tcon, + const struct drm_encoder *encoder); };
struct sun4i_tcon {
From: Andrey Lebedev andrey@lebedev.lt
A20 SoC (found in Cubieboard 2 among others) requires different LVDS set up procedure than A33. Timing controller (tcon) driver only implements sun6i-style procedure, that doesn't work on A20 (sun7i).
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 26 ++++++++++++++++++++++++++ drivers/gpu/drm/sun4i/sun4i_tcon.h | 11 +++++++++++ 2 files changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index cc6b05ca2c69..800a9bd86112 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -114,6 +114,30 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel, } }
+static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon, + const struct drm_encoder *encoder) +{ + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN4I_TCON0_LVDS_ANA0_CK_EN | + SUN4I_TCON0_LVDS_ANA0_REG_V | + SUN4I_TCON0_LVDS_ANA0_REG_C | + SUN4I_TCON0_LVDS_ANA0_EN_MB | + SUN4I_TCON0_LVDS_ANA0_PD | + SUN4I_TCON0_LVDS_ANA0_DCHS); + + udelay(2); /* delay at least 1200 ns */ + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, + SUN4I_TCON0_LVDS_ANA1_INIT, + SUN4I_TCON0_LVDS_ANA1_INIT); + udelay(1); /* delay at least 120 ns */ + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, + SUN4I_TCON0_LVDS_ANA1_UPDATE, + SUN4I_TCON0_LVDS_ANA1_UPDATE); + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN4I_TCON0_LVDS_ANA0_EN_MB, + SUN4I_TCON0_LVDS_ANA0_EN_MB); +} + static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon, const struct drm_encoder *encoder) { @@ -1456,11 +1480,13 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { };
static const struct sun4i_tcon_quirks sun7i_a20_quirks = { + .supports_lvds = true, .has_channel_0 = true, .has_channel_1 = true, .dclk_min_div = 4, /* Same display pipeline structure as A10 */ .set_mux = sun4i_a10_tcon_set_mux, + .setup_lvds_phy = sun4i_tcon_setup_lvds_phy, };
static const struct sun4i_tcon_quirks sun8i_a33_quirks = { diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index 2974e59ef9f2..cfbf4e6c1679 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -193,6 +193,13 @@ #define SUN4I_TCON_MUX_CTRL_REG 0x200
#define SUN4I_TCON0_LVDS_ANA0_REG 0x220 +#define SUN4I_TCON0_LVDS_ANA0_DCHS BIT(16) +#define SUN4I_TCON0_LVDS_ANA0_PD (BIT(20) | BIT(21)) +#define SUN4I_TCON0_LVDS_ANA0_EN_MB BIT(22) +#define SUN4I_TCON0_LVDS_ANA0_REG_C (BIT(24) | BIT(25)) +#define SUN4I_TCON0_LVDS_ANA0_REG_V (BIT(26) | BIT(27)) +#define SUN4I_TCON0_LVDS_ANA0_CK_EN (BIT(29) | BIT(28)) + #define SUN6I_TCON0_LVDS_ANA0_EN_MB BIT(31) #define SUN6I_TCON0_LVDS_ANA0_EN_LDO BIT(30) #define SUN6I_TCON0_LVDS_ANA0_EN_DRVC BIT(24) @@ -201,6 +208,10 @@ #define SUN6I_TCON0_LVDS_ANA0_V(x) (((x) & 3) << 8) #define SUN6I_TCON0_LVDS_ANA0_PD(x) (((x) & 3) << 4)
+#define SUN4I_TCON0_LVDS_ANA1_REG 0x224 +#define SUN4I_TCON0_LVDS_ANA1_INIT (0x1f << 26 | 0x1f << 10) +#define SUN4I_TCON0_LVDS_ANA1_UPDATE (0x1f << 16 | 0x1f << 00) + #define SUN4I_TCON1_FILL_CTL_REG 0x300 #define SUN4I_TCON1_FILL_BEG0_REG 0x304 #define SUN4I_TCON1_FILL_END0_REG 0x308
From: Andrey Lebedev andrey@lebedev.lt
Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and provide sample LVDS panel, connected to tcon0.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- arch/arm/boot/dts/sun7i-a20.dtsi | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 92b5be97085d..3b3c366a2bee 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -47,6 +47,7 @@ #include <dt-bindings/dma/sun4i-a10.h> #include <dt-bindings/clock/sun7i-a20-ccu.h> #include <dt-bindings/reset/sun4i-a10-ccu.h> +#include <dt-bindings/pinctrl/sun4i-a10.h>
/ { interrupt-parent = <&gic>; @@ -407,8 +408,8 @@ compatible = "allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; - resets = <&ccu RST_TCON0>; - reset-names = "lcd"; + resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; + reset-names = "lcd", "lvds"; clocks = <&ccu CLK_AHB_LCD0>, <&ccu CLK_TCON0_CH0>, <&ccu CLK_TCON0_CH1>; @@ -444,6 +445,11 @@ #size-cells = <0>; reg = <1>;
+ tcon0_out_lvds: endpoint@0 { + reg = <0>; + allwinner,tcon-channel = <0>; + }; + tcon0_out_hdmi: endpoint@1 { reg = <1>; remote-endpoint = <&hdmi_in_tcon0>; @@ -1162,6 +1168,24 @@ pins = "PI20", "PI21"; function = "uart7"; }; + + /omit-if-no-ref/ + lcd_lvds0_pins: lcd-lvds0-pins { + pins = + "PD0", "PD1", "PD2", "PD3", "PD4", + "PD5", "PD6", "PD7", "PD8", "PD9"; + function = "lvds0"; + allwinner,drive = <SUN4I_PINCTRL_10_MA>; + }; + + /omit-if-no-ref/ + lcd_lvds1_pins: lcd-lvds1-pins { + pins = + "PD10", "PD11", "PD12", "PD13", "PD14", + "PD15", "PD16", "PD17", "PD18", "PD19"; + function = "lvds1"; + allwinner,drive = <SUN4I_PINCTRL_10_MA>; + }; };
timer@1c20c00 {
On Thu, Feb 13, 2020 at 10:18:57PM +0200, Andrey Lebedev wrote:
From: Andrey Lebedev andrey@lebedev.lt
Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and provide sample LVDS panel, connected to tcon0.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
arch/arm/boot/dts/sun7i-a20.dtsi | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 92b5be97085d..3b3c366a2bee 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -47,6 +47,7 @@ #include <dt-bindings/dma/sun4i-a10.h> #include <dt-bindings/clock/sun7i-a20-ccu.h> #include <dt-bindings/reset/sun4i-a10-ccu.h> +#include <dt-bindings/pinctrl/sun4i-a10.h>
/ { interrupt-parent = <&gic>; @@ -407,8 +408,8 @@ compatible = "allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
resets = <&ccu RST_TCON0>;reset-names = "lcd";
resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;reset-names = "lcd", "lvds"; clocks = <&ccu CLK_AHB_LCD0>, <&ccu CLK_TCON0_CH0>, <&ccu CLK_TCON0_CH1>;@@ -444,6 +445,11 @@ #size-cells = <0>; reg = <1>;
tcon0_out_lvds: endpoint@0 {reg = <0>;allwinner,tcon-channel = <0>;};tcon0_out_hdmi: endpoint@1 { reg = <1>; remote-endpoint = <&hdmi_in_tcon0>;@@ -1162,6 +1168,24 @@ pins = "PI20", "PI21"; function = "uart7"; };
/omit-if-no-ref/lcd_lvds0_pins: lcd-lvds0-pins {
The nodes here should be ordered by alphabetical order
pins =
I'm not sure why you need a new line here
"PD0", "PD1", "PD2", "PD3", "PD4","PD5", "PD6", "PD7", "PD8", "PD9";function = "lvds0";allwinner,drive = <SUN4I_PINCTRL_10_MA>;
And allwinner,drive is also deprecated and at its default value anyway
Maxime
Address all outstanding review comments.
Maxime, please confirm I've got "document the new compatibles" part right.
From: Andrey Lebedev andrey@lebedev.lt
Different sunxi flavors require slightly different sequence for enabling LVDS output. This allows to differentiate between them.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 68 ++++++++++++++++-------------- drivers/gpu/drm/sun4i/sun4i_tcon.h | 3 ++ 2 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index c81cdce6ed55..cc6b05ca2c69 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -114,46 +114,48 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel, } }
+static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon, + const struct drm_encoder *encoder) +{ + u8 val; + + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_C(2) | + SUN6I_TCON0_LVDS_ANA0_V(3) | + SUN6I_TCON0_LVDS_ANA0_PD(2) | + SUN6I_TCON0_LVDS_ANA0_EN_LDO); + udelay(2); + + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_EN_MB, + SUN6I_TCON0_LVDS_ANA0_EN_MB); + udelay(2); + + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_EN_DRVC, + SUN6I_TCON0_LVDS_ANA0_EN_DRVC); + + if (sun4i_tcon_get_pixel_depth(encoder) == 18) + val = 7; + else + val = 0xf; + + regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf), + SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val)); + +} + static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon, const struct drm_encoder *encoder, bool enabled) { if (enabled) { - u8 val; - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG, SUN4I_TCON0_LVDS_IF_EN, SUN4I_TCON0_LVDS_IF_EN); - - /* - * As their name suggest, these values only apply to the A31 - * and later SoCs. We'll have to rework this when merging - * support for the older SoCs. - */ - regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_C(2) | - SUN6I_TCON0_LVDS_ANA0_V(3) | - SUN6I_TCON0_LVDS_ANA0_PD(2) | - SUN6I_TCON0_LVDS_ANA0_EN_LDO); - udelay(2); - - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_EN_MB, - SUN6I_TCON0_LVDS_ANA0_EN_MB); - udelay(2); - - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_EN_DRVC, - SUN6I_TCON0_LVDS_ANA0_EN_DRVC); - - if (sun4i_tcon_get_pixel_depth(encoder) == 18) - val = 7; - else - val = 0xf; - - regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, - SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf), - SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val)); + if (tcon->quirks->setup_lvds_phy) + tcon->quirks->setup_lvds_phy(tcon, encoder); } else { regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG, SUN4I_TCON0_LVDS_IF_EN, 0); @@ -1465,12 +1467,14 @@ static const struct sun4i_tcon_quirks sun8i_a33_quirks = { .has_channel_0 = true, .has_lvds_alt = true, .dclk_min_div = 1, + .setup_lvds_phy = sun6i_tcon_setup_lvds_phy, };
static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = { .supports_lvds = true, .has_channel_0 = true, .dclk_min_div = 1, + .setup_lvds_phy = sun6i_tcon_setup_lvds_phy, };
static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = { diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index a62ec826ae71..2974e59ef9f2 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -228,6 +228,9 @@ struct sun4i_tcon_quirks {
/* callback to handle tcon muxing options */ int (*set_mux)(struct sun4i_tcon *, const struct drm_encoder *); + /* handler for LVDS setup routine */ + void (*setup_lvds_phy)(struct sun4i_tcon *tcon, + const struct drm_encoder *encoder); };
struct sun4i_tcon {
On Wed, Feb 19, 2020 at 08:08:54PM +0200, Andrey Lebedev wrote:
From: Andrey Lebedev andrey@lebedev.lt
Different sunxi flavors require slightly different sequence for enabling LVDS output. This allows to differentiate between them.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
drivers/gpu/drm/sun4i/sun4i_tcon.c | 68 ++++++++++++++++-------------- drivers/gpu/drm/sun4i/sun4i_tcon.h | 3 ++ 2 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index c81cdce6ed55..cc6b05ca2c69 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -114,46 +114,48 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel, } }
+static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
const struct drm_encoder *encoder)+{
- u8 val;
- regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN6I_TCON0_LVDS_ANA0_C(2) |SUN6I_TCON0_LVDS_ANA0_V(3) |SUN6I_TCON0_LVDS_ANA0_PD(2) |SUN6I_TCON0_LVDS_ANA0_EN_LDO);- udelay(2);
- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN6I_TCON0_LVDS_ANA0_EN_MB,SUN6I_TCON0_LVDS_ANA0_EN_MB);- udelay(2);
- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN6I_TCON0_LVDS_ANA0_EN_DRVC,SUN6I_TCON0_LVDS_ANA0_EN_DRVC);- if (sun4i_tcon_get_pixel_depth(encoder) == 18)
val = 7;- else
val = 0xf;- regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf),SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val));+}
There's an extra blank line here that was reported by checkpatch. I've fixed it up while applying.
Maxime
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!
From: Andrey Lebedev andrey@lebedev.lt
Timing controllers on A20 are not equivalent: tcon0 on A20 supports LVDS output and tcon1 does not. Separate the capabilities by introducing independent set of quirks for each of the tcons.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index cc6b05ca2c69..b7234eef3c7b 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1508,6 +1508,8 @@ const struct of_device_id sun4i_tcon_of_table[] = { { .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks }, { .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks }, { .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks }, + { .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_quirks }, + { .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_quirks }, { .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks }, { .compatible = "allwinner,sun8i-a23-tcon", .data = &sun8i_a33_quirks }, { .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks },
On Wed, Feb 19, 2020 at 08:08:55PM +0200, Andrey Lebedev wrote:
From: Andrey Lebedev andrey@lebedev.lt
Timing controllers on A20 are not equivalent: tcon0 on A20 supports LVDS output and tcon1 does not. Separate the capabilities by introducing independent set of quirks for each of the tcons.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index cc6b05ca2c69..b7234eef3c7b 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1508,6 +1508,8 @@ const struct of_device_id sun4i_tcon_of_table[] = { { .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks }, { .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks }, { .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks },
- { .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_quirks },
- { .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_quirks },
It wasn't ordered alphabetically, I've fixed it up while applying.
Maxime
From: Andrey Lebedev andrey@lebedev.lt
Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and provide sample LVDS panel, connected to tcon0.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- arch/arm/boot/dts/sun7i-a20.dtsi | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 92b5be97085d..d50263c1ca9a 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -47,6 +47,7 @@ #include <dt-bindings/dma/sun4i-a10.h> #include <dt-bindings/clock/sun7i-a20-ccu.h> #include <dt-bindings/reset/sun4i-a10-ccu.h> +#include <dt-bindings/pinctrl/sun4i-a10.h>
/ { interrupt-parent = <&gic>; @@ -404,11 +405,12 @@ };
tcon0: lcd-controller@1c0c000 { - compatible = "allwinner,sun7i-a20-tcon"; + compatible = "allwinner,sun7i-a20-tcon0", + "allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; - resets = <&ccu RST_TCON0>; - reset-names = "lcd"; + resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; + reset-names = "lcd", "lvds"; clocks = <&ccu CLK_AHB_LCD0>, <&ccu CLK_TCON0_CH0>, <&ccu CLK_TCON0_CH1>; @@ -444,6 +446,11 @@ #size-cells = <0>; reg = <1>;
+ tcon0_out_lvds: endpoint@0 { + reg = <0>; + allwinner,tcon-channel = <0>; + }; + tcon0_out_hdmi: endpoint@1 { reg = <1>; remote-endpoint = <&hdmi_in_tcon0>; @@ -454,7 +461,8 @@ };
tcon1: lcd-controller@1c0d000 { - compatible = "allwinner,sun7i-a20-tcon"; + compatible = "allwinner,sun7i-a20-tcon1", + "allwinner,sun7i-a20-tcon"; reg = <0x01c0d000 0x1000>; interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>; resets = <&ccu RST_TCON1>; @@ -931,6 +939,20 @@ function = "ir1"; };
+ /omit-if-no-ref/ + lcd_lvds0_pins: lcd-lvds0-pins { + pins = "PD0", "PD1", "PD2", "PD3", "PD4", + "PD5", "PD6", "PD7", "PD8", "PD9"; + function = "lvds0"; + }; + + /omit-if-no-ref/ + lcd_lvds1_pins: lcd-lvds1-pins { + pins = "PD10", "PD11", "PD12", "PD13", "PD14", + "PD15", "PD16", "PD17", "PD18", "PD19"; + function = "lvds1"; + }; + /omit-if-no-ref/ mmc0_pins: mmc0-pins { pins = "PF0", "PF1", "PF2",
On Wed, Feb 19, 2020 at 08:08:56PM +0200, Andrey Lebedev wrote:
From: Andrey Lebedev andrey@lebedev.lt
Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and provide sample LVDS panel, connected to tcon0.
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
arch/arm/boot/dts/sun7i-a20.dtsi | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 92b5be97085d..d50263c1ca9a 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -47,6 +47,7 @@ #include <dt-bindings/dma/sun4i-a10.h> #include <dt-bindings/clock/sun7i-a20-ccu.h> #include <dt-bindings/reset/sun4i-a10-ccu.h> +#include <dt-bindings/pinctrl/sun4i-a10.h>
/ { interrupt-parent = <&gic>; @@ -404,11 +405,12 @@ };
tcon0: lcd-controller@1c0c000 {
compatible = "allwinner,sun7i-a20-tcon";
compatible = "allwinner,sun7i-a20-tcon0","allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
resets = <&ccu RST_TCON0>;reset-names = "lcd";
resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>;reset-names = "lcd", "lvds"; clocks = <&ccu CLK_AHB_LCD0>, <&ccu CLK_TCON0_CH0>, <&ccu CLK_TCON0_CH1>;@@ -444,6 +446,11 @@ #size-cells = <0>; reg = <1>;
tcon0_out_lvds: endpoint@0 {reg = <0>;allwinner,tcon-channel = <0>;};
This isn't necessarily true. The endpoint would be the same for an RGB panel for example. I've followed what we're doing elsewhere and removed that endpoint entirely while applying, thanks! Maxime
From: Andrey Lebedev andrey@lebedev.lt
Document new compatibles used to differentiate between timing controllers on A20 (sun7i)
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- .../bindings/display/allwinner,sun4i-a10-tcon.yaml | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml index 86ad617d2327..c0f6bb16fa34 100644 --- a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml +++ b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml @@ -46,6 +46,12 @@ properties: - allwinner,sun50i-h6-tcon-tv - const: allwinner,sun8i-a83t-tcon-tv
+ - items: + - enum: + - allwinner,sun7i-a20-tcon0 + - allwinner,sun7i-a20-tcon1 + - const: allwinner,sun7i-a20-tcon + reg: maxItems: 1
On Wed, Feb 19, 2020 at 08:08:57PM +0200, Andrey Lebedev wrote:
From: Andrey Lebedev andrey@lebedev.lt
Document new compatibles used to differentiate between timing controllers on A20 (sun7i)
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
.../bindings/display/allwinner,sun4i-a10-tcon.yaml | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml index 86ad617d2327..c0f6bb16fa34 100644 --- a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml +++ b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml @@ -46,6 +46,12 @@ properties: - allwinner,sun50i-h6-tcon-tv - const: allwinner,sun8i-a83t-tcon-tv
- items:- enum:- allwinner,sun7i-a20-tcon0- allwinner,sun7i-a20-tcon1- const: allwinner,sun7i-a20-tcon- reg: maxItems: 1
It wasn't ordered propertly, I've fixed it up while applying
Maxime
From: Andrey Lebedev andrey@lebedev.lt
A20 SoC (found in Cubieboard 2 among others) requires different LVDS set up procedure than A33. Timing controller (tcon) driver only implements sun6i-style procedure, that doesn't work on A20 (sun7i).
Signed-off-by: Andrey Lebedev andrey@lebedev.lt --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 37 +++++++++++++++++++++++++++++- drivers/gpu/drm/sun4i/sun4i_tcon.h | 11 +++++++++ 2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index b7234eef3c7b..09ee6e8c6914 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -114,6 +114,30 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel, } }
+static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon, + const struct drm_encoder *encoder) +{ + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN4I_TCON0_LVDS_ANA0_CK_EN | + SUN4I_TCON0_LVDS_ANA0_REG_V | + SUN4I_TCON0_LVDS_ANA0_REG_C | + SUN4I_TCON0_LVDS_ANA0_EN_MB | + SUN4I_TCON0_LVDS_ANA0_PD | + SUN4I_TCON0_LVDS_ANA0_DCHS); + + udelay(2); /* delay at least 1200 ns */ + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, + SUN4I_TCON0_LVDS_ANA1_INIT, + SUN4I_TCON0_LVDS_ANA1_INIT); + udelay(1); /* delay at least 120 ns */ + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, + SUN4I_TCON0_LVDS_ANA1_UPDATE, + SUN4I_TCON0_LVDS_ANA1_UPDATE); + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, + SUN4I_TCON0_LVDS_ANA0_EN_MB, + SUN4I_TCON0_LVDS_ANA0_EN_MB); +} + static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon, const struct drm_encoder *encoder) { @@ -1455,7 +1479,18 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { .dclk_min_div = 1, };
+static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = { + .supports_lvds = true, + .has_channel_0 = true, + .has_channel_1 = true, + .dclk_min_div = 4, + /* Same display pipeline structure as A10 */ + .set_mux = sun4i_a10_tcon_set_mux, + .setup_lvds_phy = sun4i_tcon_setup_lvds_phy, +}; + static const struct sun4i_tcon_quirks sun7i_a20_quirks = { + .supports_lvds = false, .has_channel_0 = true, .has_channel_1 = true, .dclk_min_div = 4, @@ -1508,7 +1543,7 @@ const struct of_device_id sun4i_tcon_of_table[] = { { .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks }, { .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks }, { .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks }, - { .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_quirks }, + { .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_tcon0_quirks }, { .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_quirks }, { .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks }, { .compatible = "allwinner,sun8i-a23-tcon", .data = &sun8i_a33_quirks }, diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index 2974e59ef9f2..cfbf4e6c1679 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -193,6 +193,13 @@ #define SUN4I_TCON_MUX_CTRL_REG 0x200
#define SUN4I_TCON0_LVDS_ANA0_REG 0x220 +#define SUN4I_TCON0_LVDS_ANA0_DCHS BIT(16) +#define SUN4I_TCON0_LVDS_ANA0_PD (BIT(20) | BIT(21)) +#define SUN4I_TCON0_LVDS_ANA0_EN_MB BIT(22) +#define SUN4I_TCON0_LVDS_ANA0_REG_C (BIT(24) | BIT(25)) +#define SUN4I_TCON0_LVDS_ANA0_REG_V (BIT(26) | BIT(27)) +#define SUN4I_TCON0_LVDS_ANA0_CK_EN (BIT(29) | BIT(28)) + #define SUN6I_TCON0_LVDS_ANA0_EN_MB BIT(31) #define SUN6I_TCON0_LVDS_ANA0_EN_LDO BIT(30) #define SUN6I_TCON0_LVDS_ANA0_EN_DRVC BIT(24) @@ -201,6 +208,10 @@ #define SUN6I_TCON0_LVDS_ANA0_V(x) (((x) & 3) << 8) #define SUN6I_TCON0_LVDS_ANA0_PD(x) (((x) & 3) << 4)
+#define SUN4I_TCON0_LVDS_ANA1_REG 0x224 +#define SUN4I_TCON0_LVDS_ANA1_INIT (0x1f << 26 | 0x1f << 10) +#define SUN4I_TCON0_LVDS_ANA1_UPDATE (0x1f << 16 | 0x1f << 00) + #define SUN4I_TCON1_FILL_CTL_REG 0x300 #define SUN4I_TCON1_FILL_BEG0_REG 0x304 #define SUN4I_TCON1_FILL_END0_REG 0x308
On Wed, Feb 19, 2020 at 08:08:58PM +0200, Andrey Lebedev wrote:
From: Andrey Lebedev andrey@lebedev.lt
A20 SoC (found in Cubieboard 2 among others) requires different LVDS set up procedure than A33. Timing controller (tcon) driver only implements sun6i-style procedure, that doesn't work on A20 (sun7i).
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
drivers/gpu/drm/sun4i/sun4i_tcon.c | 37 +++++++++++++++++++++++++++++- drivers/gpu/drm/sun4i/sun4i_tcon.h | 11 +++++++++ 2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index b7234eef3c7b..09ee6e8c6914 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -114,6 +114,30 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel, } }
+static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
const struct drm_encoder *encoder)+{
- regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN4I_TCON0_LVDS_ANA0_CK_EN |SUN4I_TCON0_LVDS_ANA0_REG_V |SUN4I_TCON0_LVDS_ANA0_REG_C |SUN4I_TCON0_LVDS_ANA0_EN_MB |SUN4I_TCON0_LVDS_ANA0_PD |SUN4I_TCON0_LVDS_ANA0_DCHS);- udelay(2); /* delay at least 1200 ns */
- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
SUN4I_TCON0_LVDS_ANA1_INIT,SUN4I_TCON0_LVDS_ANA1_INIT);- udelay(1); /* delay at least 120 ns */
- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
SUN4I_TCON0_LVDS_ANA1_UPDATE,SUN4I_TCON0_LVDS_ANA1_UPDATE);- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN4I_TCON0_LVDS_ANA0_EN_MB,SUN4I_TCON0_LVDS_ANA0_EN_MB);+}
static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon, const struct drm_encoder *encoder) { @@ -1455,7 +1479,18 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { .dclk_min_div = 1, };
+static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = {
- .supports_lvds = true,
- .has_channel_0 = true,
- .has_channel_1 = true,
- .dclk_min_div = 4,
- /* Same display pipeline structure as A10 */
- .set_mux = sun4i_a10_tcon_set_mux,
- .setup_lvds_phy = sun4i_tcon_setup_lvds_phy,
+};
static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
- .supports_lvds = false,
False is already the default here.
I've removed it while applying
Maxime
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 andrey@lebedev.lt
A20 SoC (found in Cubieboard 2 among others) requires different LVDS set up procedure than A33. Timing controller (tcon) driver only implements sun6i-style procedure, that doesn't work on A20 (sun7i).
Signed-off-by: Andrey Lebedev andrey@lebedev.lt
drivers/gpu/drm/sun4i/sun4i_tcon.c | 37 +++++++++++++++++++++++++++++- drivers/gpu/drm/sun4i/sun4i_tcon.h | 11 +++++++++ 2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index b7234eef3c7b..09ee6e8c6914 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -114,6 +114,30 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel, } }
+static void sun4i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon,
const struct drm_encoder *encoder)+{
- regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN4I_TCON0_LVDS_ANA0_CK_EN |SUN4I_TCON0_LVDS_ANA0_REG_V |SUN4I_TCON0_LVDS_ANA0_REG_C |SUN4I_TCON0_LVDS_ANA0_EN_MB |SUN4I_TCON0_LVDS_ANA0_PD |SUN4I_TCON0_LVDS_ANA0_DCHS);- udelay(2); /* delay at least 1200 ns */
- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
SUN4I_TCON0_LVDS_ANA1_INIT,SUN4I_TCON0_LVDS_ANA1_INIT);- udelay(1); /* delay at least 120 ns */
- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG,
SUN4I_TCON0_LVDS_ANA1_UPDATE,SUN4I_TCON0_LVDS_ANA1_UPDATE);- regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG,
SUN4I_TCON0_LVDS_ANA0_EN_MB,SUN4I_TCON0_LVDS_ANA0_EN_MB);+}
- static void sun6i_tcon_setup_lvds_phy(struct sun4i_tcon *tcon, const struct drm_encoder *encoder) {
@@ -1455,7 +1479,18 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { .dclk_min_div = 1, };
+static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = {
- .supports_lvds = true,
- .has_channel_0 = true,
- .has_channel_1 = true,
- .dclk_min_div = 4,
- /* Same display pipeline structure as A10 */
- .set_mux = sun4i_a10_tcon_set_mux,
- .setup_lvds_phy = sun4i_tcon_setup_lvds_phy,
+};
- static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
- .supports_lvds = false,
False is already the default here.
I've removed it while applying
Maxime
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+QW...
Maxime
dri-devel@lists.freedesktop.org
-
Andrey Lebedev -
Andrey Lebedev -
andrey.lebedev＠gmail.com
-
Maxime Ripard