| Message ID | 20200210195633.GA21832@kedthinkpad (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] Support LVDS output on Allwinner A20 | expand |
Hi, On Mon, Feb 10, 2020 at 09:56:33PM +0200, Andrey Lebedev wrote: > A20 SoC (found in Cubieboard 2 among others) requires different LVDS set > up procedure than A33. Timing controller (tcon) driver only implements > sun6i-style procedure, that doesn't work on A20 (sun7i). You're missing your Signed-off-by here. > The support for such procedure is ported from u-boot and follows u-boot > naming convention: SUN6I* for sun6i-style procedure, and SUN4I for other > (which happens to be compatible with A20). A commit log is mostly about why you're doing a change, this part can be left out. > --- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 91 ++++++++++++++++++++---------- > drivers/gpu/drm/sun4i/sun4i_tcon.h | 12 ++++ > 2 files changed, 73 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index c81cdce6ed55..78896e907ca9 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -114,46 +114,74 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel, > } > } > > +static void sun4i_tcon_lvds_sun6i_enable(struct sun4i_tcon *tcon, > + const struct drm_encoder *encoder) { This doesn't match the kernel coding style, make sure to run checkpatch. Also, using something like sun6i_tcon_setup_lvds_phy would be more fit to what this function is doing. > + u8 val; > + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, > + SUN6I_TCON0_LVDS_ANA0_C(2) | > + SUN6I_TCON0_LVDS_ANA0_V(3) | > + SUN6I_TCON0_LVDS_ANA0_PD(2) | > + SUN6I_TCON0_LVDS_ANA0_EN_LDO); > + udelay(2); > + > + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, > + SUN6I_TCON0_LVDS_ANA0_EN_MB, > + SUN6I_TCON0_LVDS_ANA0_EN_MB); > + udelay(2); > + > + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, > + SUN6I_TCON0_LVDS_ANA0_EN_DRVC, > + SUN6I_TCON0_LVDS_ANA0_EN_DRVC); > + > + if (sun4i_tcon_get_pixel_depth(encoder) == 18) > + val = 7; > + else > + val = 0xf; > + > + regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, > + SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf), > + SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val)); > + > +} > + > +static void sun4i_tcon_lvds_sun4i_enable(struct sun4i_tcon *tcon) { And sun4i_tcon_setup_lvds_phy. > + regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, > + SUN4I_TCON0_LVDS_ANA0_CK_EN | > + SUN4I_TCON0_LVDS_ANA0_REG_V | > + SUN4I_TCON0_LVDS_ANA0_REG_C | > + SUN4I_TCON0_LVDS_ANA0_EN_MB | > + SUN4I_TCON0_LVDS_ANA0_PD | > + SUN4I_TCON0_LVDS_ANA0_DCHS); > + > + udelay(2); /* delay at least 1200 ns */ > + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, > + SUN4I_TCON0_LVDS_ANA1_INIT, > + SUN4I_TCON0_LVDS_ANA1_INIT); > + udelay(1); /* delay at least 1200 ns */ The delay and your comment don't match. > + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, > + SUN4I_TCON0_LVDS_ANA1_UPDATE, > + SUN4I_TCON0_LVDS_ANA1_UPDATE); You refer to U-Boot in your commit log, but the sequence is not quite the same, why did you change it? > +} > + > + > static void sun4i_tcon_lvds_set_status(struct sun4i_tcon *tcon, > const struct drm_encoder *encoder, > bool enabled) > { > if (enabled) { > - u8 val; > - > + // Enable LVDS interface There's no need for that comment, it's simple enough :) > regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG, > SUN4I_TCON0_LVDS_IF_EN, > SUN4I_TCON0_LVDS_IF_EN); > > - /* > - * As their name suggest, these values only apply to the A31 > - * and later SoCs. We'll have to rework this when merging > - * support for the older SoCs. > - */ > - regmap_write(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, > - SUN6I_TCON0_LVDS_ANA0_C(2) | > - SUN6I_TCON0_LVDS_ANA0_V(3) | > - SUN6I_TCON0_LVDS_ANA0_PD(2) | > - SUN6I_TCON0_LVDS_ANA0_EN_LDO); > - udelay(2); > - > - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, > - SUN6I_TCON0_LVDS_ANA0_EN_MB, > - SUN6I_TCON0_LVDS_ANA0_EN_MB); > - udelay(2); > - > - regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, > - SUN6I_TCON0_LVDS_ANA0_EN_DRVC, > - SUN6I_TCON0_LVDS_ANA0_EN_DRVC); > - > - if (sun4i_tcon_get_pixel_depth(encoder) == 18) > - val = 7; > - else > - val = 0xf; > + // Perform SoC-specific setup procedure Ditto. > + if (tcon->quirks->sun6i_lvds_init) { > + sun4i_tcon_lvds_sun6i_enable(tcon, encoder); > + } > + else { > + sun4i_tcon_lvds_sun4i_enable(tcon); > + } > > - regmap_write_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA0_REG, > - SUN6I_TCON0_LVDS_ANA0_EN_DRVD(0xf), > - SUN6I_TCON0_LVDS_ANA0_EN_DRVD(val)); > } else { > regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_IF_REG, > SUN4I_TCON0_LVDS_IF_EN, 0); > @@ -1454,6 +1482,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { > }; > > static const struct sun4i_tcon_quirks sun7i_a20_quirks = { > + .supports_lvds = true, > .has_channel_0 = true, > .has_channel_1 = true, > .dclk_min_div = 4, > @@ -1464,11 +1493,13 @@ static const struct sun4i_tcon_quirks sun7i_a20_quirks = { > static const struct sun4i_tcon_quirks sun8i_a33_quirks = { > .has_channel_0 = true, > .has_lvds_alt = true, > + .sun6i_lvds_init = true, Using a function pointer here (like we're doing with set_mux) would be more future proof. > .dclk_min_div = 1, > }; > > static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = { > .supports_lvds = true, > + .sun6i_lvds_init = true, > .has_channel_0 = true, > .dclk_min_div = 1, > }; > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h > index a62ec826ae71..973901c1bee5 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h > @@ -193,6 +193,13 @@ > #define SUN4I_TCON_MUX_CTRL_REG 0x200 > > #define SUN4I_TCON0_LVDS_ANA0_REG 0x220 > +#define SUN4I_TCON0_LVDS_ANA0_DCHS BIT(16) > +#define SUN4I_TCON0_LVDS_ANA0_PD BIT(20) | BIT(21) > +#define SUN4I_TCON0_LVDS_ANA0_EN_MB BIT(22) > +#define SUN4I_TCON0_LVDS_ANA0_REG_C BIT(24) | BIT(25) > +#define SUN4I_TCON0_LVDS_ANA0_REG_V BIT(26) | BIT(27) > +#define SUN4I_TCON0_LVDS_ANA0_CK_EN BIT(29) | BIT(28) > + > #define SUN6I_TCON0_LVDS_ANA0_EN_MB BIT(31) > #define SUN6I_TCON0_LVDS_ANA0_EN_LDO BIT(30) > #define SUN6I_TCON0_LVDS_ANA0_EN_DRVC BIT(24) > @@ -201,6 +208,10 @@ > #define SUN6I_TCON0_LVDS_ANA0_V(x) (((x) & 3) << 8) > #define SUN6I_TCON0_LVDS_ANA0_PD(x) (((x) & 3) << 4) > > +#define SUN4I_TCON0_LVDS_ANA1_REG 0x224 > +#define SUN4I_TCON0_LVDS_ANA1_INIT (0x1f << 26 | 0x1f << 10) > +#define SUN4I_TCON0_LVDS_ANA1_UPDATE (0x1f << 16 | 0x1f << 00) Having proper defines for those fields would be great too. Side question, this will need some DT changes too, right? Maxime
Maxime, thanks for your comments. I'll update the patch, but meanwhile, I have some remarks/questions, see below. On Tue, Feb 11, 2020 at 08:20:04AM +0100, Maxime Ripard wrote: > > + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, > > + SUN4I_TCON0_LVDS_ANA1_UPDATE, > > + SUN4I_TCON0_LVDS_ANA1_UPDATE); > > You refer to U-Boot in your commit log, but the sequence is not quite > the same, why did you change it? I actually had two reference implementations at my hand. One was u-boot and another - an old (abandoned) branch of Priit Laes [1] (I took the split-up of u-boot SUNXI_LCDC_LVDS_ANA0 constant from there). This is an attempt to simplify the sequence, since I noticed that the same bit was being set to the same register twice [2] and removing that duplication didn't produce any observable regression. Priit implementation didn't have that bit set in the end of the sequence either, so I omitted it. That said, I agree that it could've been a bit naive on my side, and I can get it back to match u-boot version, if you feel that might be important. For the reference the U-Boot code is here: [3] [1] https://github.com/plaes/linux/commit/cc8c8bab2f2f2752ba6b11632dcd0f41bac249bc#diff-014a76a5007005a7a240825a972b8c7fR127 [2] setbits_le32(&lcdc->lvds_ana0, SUNXI_LCDC_LVDS_ANA0_UPDATE); [3] https://github.com/ARM-software/u-boot/blob/master/drivers/video/sunxi/lcdc.c#L60 > > +#define SUN4I_TCON0_LVDS_ANA1_REG 0x224 > > +#define SUN4I_TCON0_LVDS_ANA1_INIT (0x1f << 26 | 0x1f << 10) > > +#define SUN4I_TCON0_LVDS_ANA1_UPDATE (0x1f << 16 | 0x1f << 00) > > Having proper defines for those fields would be great too. If by "proper" you mean "split them up to individual bits", I would agree, but I can't find any actual hardware reference documentation that would mention the meaning of these registers. In both places (u-boot and Priit) these constants are defined the same way. I took the liberty to rename ANA1_INIT1 to ANA1_INIT and ANA1_INIT2 to ANA1_UPDATE to match Priit naming rather than u-boot, as I felt it was more descriptive. I have no strong opinion here though. > Side question, this will need some DT changes too, right? Hm, I agree. I think it would be reasonable to include LVDS0/1 pins and sample (but disabled) lvds panel, connected to tcon to arch/arm/boot/dts/sun7i-a20.dtsi. Does that make sense to you? Would you expect dts changes in the same patch or separate? P.S. This is my first patch to the linux kernel, please forgive me my inexperience.
Hi Andrey, On Tue, Feb 11, 2020 at 10:48:28PM +0200, Andrey Lebedev wrote: > Maxime, thanks for your comments. I'll update the patch, but meanwhile, > I have some remarks/questions, see below. > > On Tue, Feb 11, 2020 at 08:20:04AM +0100, Maxime Ripard wrote: > > > + regmap_update_bits(tcon->regs, SUN4I_TCON0_LVDS_ANA1_REG, > > > + SUN4I_TCON0_LVDS_ANA1_UPDATE, > > > + SUN4I_TCON0_LVDS_ANA1_UPDATE); > > > > You refer to U-Boot in your commit log, but the sequence is not quite > > the same, why did you change it? > > I actually had two reference implementations at my hand. One was u-boot > and another - an old (abandoned) branch of Priit Laes [1] (I took the > split-up of u-boot SUNXI_LCDC_LVDS_ANA0 constant from there). > > This is an attempt to simplify the sequence, since I noticed that the > same bit was being set to the same register twice [2] and removing that > duplication didn't produce any observable regression. Priit > implementation didn't have that bit set in the end of the sequence > either, so I omitted it. That said, I agree that it could've been a bit > naive on my side, and I can get it back to match u-boot version, if you > feel that might be important. > > For the reference the U-Boot code is here: [3] > > [1] https://github.com/plaes/linux/commit/cc8c8bab2f2f2752ba6b11632dcd0f41bac249bc#diff-014a76a5007005a7a240825a972b8c7fR127 > [2] setbits_le32(&lcdc->lvds_ana0, SUNXI_LCDC_LVDS_ANA0_UPDATE); > [3] https://github.com/ARM-software/u-boot/blob/master/drivers/video/sunxi/lcdc.c#L60 The U-Boot code has been here for a while and we know it's robust by now, so I'd prefer to be conservative and use it here. > > > +#define SUN4I_TCON0_LVDS_ANA1_REG 0x224 > > > +#define SUN4I_TCON0_LVDS_ANA1_INIT (0x1f << 26 | 0x1f << 10) > > > +#define SUN4I_TCON0_LVDS_ANA1_UPDATE (0x1f << 16 | 0x1f << 00) > > > > Having proper defines for those fields would be great too. > > If by "proper" you mean "split them up to individual bits", I would > agree, but I can't find any actual hardware reference documentation that > would mention the meaning of these registers. Of course we don't.. :) It's fine to leave them as is then > In both places (u-boot and Priit) these constants are defined the same way. > > I took the liberty to rename ANA1_INIT1 to ANA1_INIT and ANA1_INIT2 to > ANA1_UPDATE to match Priit naming rather than u-boot, as I felt it was > more descriptive. I have no strong opinion here though. > > > Side question, this will need some DT changes too, right? > > Hm, I agree. I think it would be reasonable to include LVDS0/1 pins That, but most importantly, the reset and clocks for the LVDS block. Also from looking at it, I'm not entirely sure that the TCON1 has a LVDS output, do you have a board when you have been able to test it? > and sample (but disabled) lvds panel, That's good for the sake of the example, but it shouldn't be in the same patch, it won't be merged. > connected to tcon to arch/arm/boot/dts/sun7i-a20.dtsi. Does that > make sense to you? Would you expect dts changes in the same patch or > separate? > > P.S. This is my first patch to the linux kernel, please forgive me my > inexperience. You're doing fine so far :) Maxime
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
Address all outstanding review comments. Maxime, please confirm I've got "document the new compatibles" part right.
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 */