[PATCH v2 4/5] sunxi: video: v3s: Enable LCD support

Maxime Ripard maxime at cerno.tech
Mon Oct 26 20:09:35 CET 2020


On Sun, Oct 25, 2020 at 10:46:04AM +0100, Martin Cerveny wrote:
> 
> On Mon, 19 Oct 2020, Maxime Ripard wrote:
> > On Fri, Oct 16, 2020 at 07:17:04PM +0200, Martin Cerveny wrote:
> > > Enable support for V3s LCD display with following changes:
> > > 
> > > V3s has 2x VI and 1x UI channels (use UI channel).
> > > V3s uses PLL3 (PLL_VIDEO) for both DE2 and TCON0 pixelclock.
> > > V3s does not support doubleclock for PLL3.
> > > V3s supports resolution upto 1024x1024.
> > > V3s does not support HDMI.
> > > 
> > > Signed-off-by: Martin Cerveny <m.cerveny at computer.org>
> > > ---
> > >  arch/arm/include/asm/arch-sunxi/clock_sun6i.h |  5 ++--
> > >  arch/arm/include/asm/arch-sunxi/gpio.h        |  1 +
> > >  arch/arm/mach-sunxi/Kconfig                   |  1 +
> > >  drivers/video/sunxi/lcdc.c                    |  5 ++--
> > >  drivers/video/sunxi/sunxi_de2.c               | 25 ++++++++++++++++---
> > >  drivers/video/sunxi/sunxi_dw_hdmi.c           |  2 ++
> > >  drivers/video/sunxi/sunxi_lcd.c               |  9 ++++++-
> > >  7 files changed, 40 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> > > index ee387127f3..9efe05d103 100644
> > > --- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> > > +++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> > > @@ -329,7 +329,7 @@ struct sunxi_ccm_reg {
> > >  #define AHB_GATE_OFFSET_DE		12
> > >  #define AHB_GATE_OFFSET_HDMI		11
> > >  #define AHB_GATE_OFFSET_TVE		9
> > > -#ifndef CONFIG_SUNXI_DE2
> > > +#if !defined(CONFIG_SUNXI_DE2) || defined(CONFIG_MACH_SUN8I_V3S)
> > >  #define AHB_GATE_OFFSET_LCD1		5
> > >  #define AHB_GATE_OFFSET_LCD0		4
> > >  #else
> > > @@ -476,7 +476,7 @@ struct sunxi_ccm_reg {
> > >  #define AHB_RESET_OFFSET_HDMI		11
> > >  #define AHB_RESET_OFFSET_HDMI2		10
> > >  #define AHB_RESET_OFFSET_TVE		9
> > > -#ifndef CONFIG_SUNXI_DE2
> > > +#if !defined(CONFIG_SUNXI_DE2) || defined(CONFIG_MACH_SUN8I_V3S)
> > 
> > These two changes are confusing. The V3S has a DE2, so having that
> > condition is weird. I'd just add an elif there
> 
> The use of "CONFIG_SUNXI_DE2" is misleading.
> The platform connectivity (AHB) depends on platform not on DE2.

Feel free to fix it :)

> Names are also weird. Should be "TCON".

That one is less weird. TCON used to be called LCDC as well in the older
SoCs.

> AHB gating is described in platform docs and not in DE2 docs.
> 
> header:
> 
> ahb_gate1;		/* 0x64 ahb module clock gating 1 */
> u32 ahb_reset1_cfg;	/* 0x2c4 AHB1 Reset 1 config */
> /* ahb_gate1 offsets */
> #define AHB_GATE_OFFSET_LCD0 ...
> /* ahb_reset1 offsets */
> #define AHB_RESET_OFFSET_LCD0
> 
> V3S datasheet:
> ("LCD0" (it is fixed used in drivers/video/sunxi/lcdc.c) is TCON (==TCON0)
> and does not have "LCD1" (==TCON1))
> 
> Offset: 0x0064 Register Name: BUS_CLK_GATING_REG1
> Bit: 4 TCON_GATING
> Offset: 0x02C4 Register Name: BUS_SOFT_RST_REG1
> Bit: 4 TCON_RST
> 
> > >  #define AHB_RESET_OFFSET_LCD1		5
> > >  #define AHB_RESET_OFFSET_LCD0		4
> > >  #else
> > > @@ -510,6 +510,7 @@ struct sunxi_ccm_reg {
> > >  #define CCM_DE2_CTRL_PLL_MASK		(3 << 24)
> > >  #define CCM_DE2_CTRL_PLL6_2X		(0 << 24)
> > >  #define CCM_DE2_CTRL_PLL10		(1 << 24)
> > > +#define CCM_DE2_CTRL_PLL3_V3S		(0 << 24)
> > >  #define CCM_DE2_CTRL_GATE		(0x1 << 31)
> > > 
> > >  /* CCU security switch, H3 only */
> > > diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
> > > index d83dfdf605..9b580fbe26 100644
> > > --- a/arch/arm/include/asm/arch-sunxi/gpio.h
> > > +++ b/arch/arm/include/asm/arch-sunxi/gpio.h
> > > @@ -181,6 +181,7 @@ enum sunxi_gpio_number {
> > >  #define SUN5I_GPE_SDC2		3
> > >  #define SUN8I_GPE_TWI2		3
> > >  #define SUN50I_GPE_TWI2		3
> > > +#define SUN8I_V3S_GPE_LCD0	3
> > > 
> > >  #define SUNXI_GPF_SDC0		2
> > >  #define SUNXI_GPF_UART0		4
> > > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > > index be0822bfb7..dc0ee2cdef 100644
> > > --- a/arch/arm/mach-sunxi/Kconfig
> > > +++ b/arch/arm/mach-sunxi/Kconfig
> > > @@ -258,6 +258,7 @@ config MACH_SUN8I_V3S
> > >  	select CPU_V7_HAS_NONSEC
> > >  	select CPU_V7_HAS_VIRT
> > >  	select ARCH_SUPPORT_PSCI
> > > +	select SUNXI_DE2
> > >  	select SUNXI_GEN_SUN6I
> > >  	select SUNXI_DRAM_DW
> > >  	select SUNXI_DRAM_DW_16BIT
> > > diff --git a/drivers/video/sunxi/lcdc.c b/drivers/video/sunxi/lcdc.c
> > > index 73033c3b85..3d50f9d567 100644
> > > --- a/drivers/video/sunxi/lcdc.c
> > > +++ b/drivers/video/sunxi/lcdc.c
> > > @@ -244,7 +244,7 @@ void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
> > >  	 * not sync to higher frequencies.
> > >  	 */
> > >  	for (m = min_m; m <= max_m; m++) {
> > > -#ifndef CONFIG_SUNXI_DE2
> > > +#if !defined(CONFIG_SUNXI_DE2) || defined(CONFIG_MACH_SUN8I_V3S)
> > >  		n = (m * dotclock) / step;
> > > 
> > >  		if ((n >= 9) && (n <= 127)) {
> > > @@ -262,7 +262,7 @@ void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
> > >  		if (!(m & 1))
> > >  			continue;
> > >  #endif
> > > -
> > > +#ifndef CONFIG_MACH_SUN8I_V3S
> > >  		/* No double clock on DE2 */
> > >  		n = (m * dotclock) / (step * 2);
> > >  		if ((n >= 9) && (n <= 127)) {
> > > @@ -275,6 +275,7 @@ void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
> > >  				best_double = 1;
> > >  			}
> > >  		}
> > > +#endif
> > 
> > I'm still not seeing any indication as to where you're getting this from
> 
> Again.
> The use of "CONFIG_SUNXI_DE2" is misleading.
> The platform PLL3 functions depends on platform not on DE2.
> 
> PLL3 on V3S platform does not support double speed.
> "best_double" should not be set.
> 
> header:
> 
> u32 lcd0_ch0_clk_cfg;	/* 0x118 LCD0 CH0 module clock */
> #define CCM_LCD_CH0_CTRL_PLL3		(0 << 24)
> #define CCM_LCD_CH0_CTRL_PLL7		(1 << 24)
> #define CCM_LCD_CH0_CTRL_PLL3_2X	(2 << 24)
> #define CCM_LCD_CH0_CTRL_PLL7_2X	(3 << 24)
> #define CCM_LCD_CH0_CTRL_MIPI_PLL	(4 << 24)
> /* No reset bit in ch0_clk_cfg (reset is controlled through ahb_reset1) */
> #define CCM_LCD_CH0_CTRL_RST		0
> #define CCM_LCD_CH0_CTRL_GATE		(0x1 << 31)
> 
> V3s datasheet:
> 
> Offset: 0x0118 Register Name: TCON_CLK_REG
> Bit: 31 SCLK_GATING
> Bit: 26:24 CLK_SRC_SEL
>    - valid values:
>        000: PLL_VIDEO (PLL3)
>        001: PLL_PERIPH0 (PLL6)
> Bit: 3:0 CLK_DIV_RATIO_M

Then that's your source. It should be in the commit log.

> > >  	}
> > > 
> > >  #ifdef CONFIG_MACH_SUN6I
> > > diff --git a/drivers/video/sunxi/sunxi_de2.c b/drivers/video/sunxi/sunxi_de2.c
> > > index b657e163f0..49d41eb243 100644
> > > --- a/drivers/video/sunxi/sunxi_de2.c
> > > +++ b/drivers/video/sunxi/sunxi_de2.c
> > > @@ -26,12 +26,21 @@
> > > 
> > >  DECLARE_GLOBAL_DATA_PTR;
> > > 
> > > +#ifdef CONFIG_MACH_SUN8I_V3S
> > > +enum {
> > > +	/* Maximum LCD size we support */
> > > +	LCD_MAX_WIDTH		= 1024,
> > > +	LCD_MAX_HEIGHT		= 1024,
> > > +	LCD_MAX_LOG2_BPP	= VIDEO_BPP32,
> > > +};
> > > +#else
> > >  enum {
> > >  	/* Maximum LCD size we support */
> > >  	LCD_MAX_WIDTH		= 3840,
> > >  	LCD_MAX_HEIGHT		= 2160,
> > >  	LCD_MAX_LOG2_BPP	= VIDEO_BPP32,
> > >  };
> > > +#endif
> > > 
> > >  static void sunxi_de2_composer_init(void)
> > >  {
> > > @@ -47,11 +56,19 @@ static void sunxi_de2_composer_init(void)
> > >  	writel(reg_value, SUNXI_SRAMC_BASE + 0x04);
> > >  #endif
> > > 
> > > +#ifdef CONFIG_MACH_SUN8I_V3S
> > > +	clock_set_pll3(50000000);
> > > +	/* pll3 is also used for pixelclock and speed will be recomputed */
> > > +	/* Set DE parent to pll3 */
> > > +	clrsetbits_le32(&ccm->de_clk_cfg, CCM_DE2_CTRL_PLL_MASK,
> > > +			CCM_DE2_CTRL_PLL3_V3S);
> > > +#else
> > >  	clock_set_pll10(432000000);
> > > 
> > >  	/* Set DE parent to pll10 */
> > >  	clrsetbits_le32(&ccm->de_clk_cfg, CCM_DE2_CTRL_PLL_MASK,
> > >  			CCM_DE2_CTRL_PLL10);
> > > +#endif
> > > 
> > >  	/* Set ahb gating to pass */
> > >  	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_DE);
> > > @@ -77,7 +94,8 @@ static void sunxi_de2_mode_set(int mux, const struct display_timing *mode,
> > >  	struct de_ui * const de_ui_regs =
> > >  		(struct de_ui *)(de_mux_base +
> > >  				 SUNXI_DE2_MUX_CHAN_REGS +
> > > -				 SUNXI_DE2_MUX_CHAN_SZ * 1);
> > > +				 SUNXI_DE2_MUX_CHAN_SZ *
> > > +				 (IS_ENABLED(CONFIG_MACH_SUN8I_V3S) ? 2 : 1));
> > 
> > Why do you need to use two UI channels? Isn't one enough?
> 
> Not "two" but I am using third (0,1,_2_) channel.
> Third channel is only UI channel on V3s platform.
> (first two channels are VI channels).
> 
> see:
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/sun4i/sun8i_mixer.c#L581

This should also be in the commit log

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201026/0d97147d/attachment.sig>


More information about the U-Boot mailing list