[PATCH 00/18] i.MXRT1050 add LCDIF support

This patchset add support for LCDIF on i.MXRT1050 evk. This requires PLL5 to be setup, mxsfb needs to use display_timing to retrieve if Lcd has inverted PIXCLOCK from dts.
With this patchset applied we temporary loose DCache support until it will get implemented, since a function in mxsfb.c is needed for setting cache behaviour. Anyway this way Lcd will show the console same way as serial does.
Also I've moved private sunxi_ctfb_mode_to_display_timing() to videomodes since I need it for mxfsb.c too, then having a unified function to convert from ctfb_mode to display_timing.
Giulio Benetti (18): clk: imx: pllv3: add enable_bit clk: imx: imxrt1050-clk: fix typo in clock name "video:" clk: imx: clk-imxrt1050: setup PLL5 for video in non-SPL videomodes: add helper function to convert from ctfb to display_timing sunxi: display: use common video_ctfb_mode_to_display_timing() video: mxsfb: add support for DM CLK video: mxsfb: add support for i.MXRT video: mxsfb: refactor for using display_timings video: mxsfb: enable setting HSYNC negative polarity video: mxsfb: enable setting VSYNC negative polarity video: mxsfb: enable setting PIXDATA on negative edge video: mxsfb: enable setting ENABLE negative polarity imxrt1050_evk: add 16bpp video support if video layer enabled ARM: dts: i.mxrt1050: add lcdif node ARM: dts: imxrt1050: allow this dtsi file to be compiled in Linux arch: arm: dts: imxrt1050-evk: add lcdif node configs: imxrt1050-evk: enable video support/console configs: imxrt1050-evk: temporary disable DCACHE
arch/arm/dts/imxrt1050-evk.dts | 57 +++++++++++++ arch/arm/dts/imxrt1050.dtsi | 14 +++- arch/arm/include/asm/arch-imxrt/imx-regs.h | 6 ++ arch/arm/include/asm/mach-imx/regs-lcdif.h | 6 +- configs/imxrt1050-evk_defconfig | 6 ++ drivers/clk/imx/clk-imxrt1050.c | 15 +++- drivers/clk/imx/clk-pllv3.c | 9 +++ drivers/video/mxsfb.c | 94 ++++++++++++++-------- drivers/video/sunxi/sunxi_display.c | 33 +------- drivers/video/videomodes.c | 29 +++++++ drivers/video/videomodes.h | 3 + include/configs/imxrt1050-evk.h | 15 ++++ 12 files changed, 216 insertions(+), 71 deletions(-)

pllv3 PLLs have powerdown/up bits but enable bits too. Specifically "enable bit" enable the pll output, so when dis/enabling pll by setting/clearing power_bit we must also set/clear enable_bit.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/clk/imx/clk-pllv3.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c index 525442debf..b4a9d587e1 100644 --- a/drivers/clk/imx/clk-pllv3.c +++ b/drivers/clk/imx/clk-pllv3.c @@ -25,6 +25,7 @@ #define PLL_DENOM_OFFSET 0x20
#define BM_PLL_POWER (0x1 << 12) +#define BM_PLL_ENABLE (0x1 << 13) #define BM_PLL_LOCK (0x1 << 31)
struct clk_pllv3 { @@ -32,6 +33,7 @@ struct clk_pllv3 { void __iomem *base; u32 power_bit; bool powerup_set; + u32 enable_bit; u32 div_mask; u32 div_shift; }; @@ -83,6 +85,9 @@ static int clk_pllv3_generic_enable(struct clk *clk) val |= pll->power_bit; else val &= ~pll->power_bit; + + val |= pll->enable_bit; + writel(val, pll->base);
return 0; @@ -98,6 +103,9 @@ static int clk_pllv3_generic_disable(struct clk *clk) val &= ~pll->power_bit; else val |= pll->power_bit; + + val &= ~pll->enable_bit; + writel(val, pll->base);
return 0; @@ -238,6 +246,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name, return ERR_PTR(-ENOMEM);
pll->power_bit = BM_PLL_POWER; + pll->enable_bit = BM_PLL_ENABLE;
switch (type) { case IMX_PLLV3_GENERIC:

On Wed, 26 Feb 2020 18:15:44 +0100 Giulio Benetti giulio.benetti@benettiengineering.com wrote:
pllv3 PLLs have powerdown/up bits but enable bits too. Specifically "enable bit" enable the pll output, so when dis/enabling pll by setting/clearing power_bit we must also set/clear enable_bit.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com
drivers/clk/imx/clk-pllv3.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c index 525442debf..b4a9d587e1 100644 --- a/drivers/clk/imx/clk-pllv3.c +++ b/drivers/clk/imx/clk-pllv3.c @@ -25,6 +25,7 @@ #define PLL_DENOM_OFFSET 0x20
#define BM_PLL_POWER (0x1 << 12) +#define BM_PLL_ENABLE (0x1 << 13) #define BM_PLL_LOCK (0x1 << 31)
struct clk_pllv3 { @@ -32,6 +33,7 @@ struct clk_pllv3 { void __iomem *base; u32 power_bit; bool powerup_set;
- u32 enable_bit; u32 div_mask; u32 div_shift;
}; @@ -83,6 +85,9 @@ static int clk_pllv3_generic_enable(struct clk *clk) val |= pll->power_bit; else val &= ~pll->power_bit;
val |= pll->enable_bit;
writel(val, pll->base);
return 0;
@@ -98,6 +103,9 @@ static int clk_pllv3_generic_disable(struct clk *clk) val &= ~pll->power_bit; else val |= pll->power_bit;
val &= ~pll->enable_bit;
writel(val, pll->base);
return 0;
@@ -238,6 +246,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name, return ERR_PTR(-ENOMEM);
pll->power_bit = BM_PLL_POWER;
pll->enable_bit = BM_PLL_ENABLE;
switch (type) { case IMX_PLLV3_GENERIC:
Reviewed-by: Lukasz Majewski lukma@denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

"video:" must be "video", ":" is a typo.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/clk/imx/clk-imxrt1050.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imxrt1050.c b/drivers/clk/imx/clk-imxrt1050.c index 44ca52c013..e33d426363 100644 --- a/drivers/clk/imx/clk-imxrt1050.c +++ b/drivers/clk/imx/clk-imxrt1050.c @@ -90,7 +90,7 @@ static const char *const usdhc_sels[] = { "pll2_pfd2_396m", "pll2_pfd0_352m", }; static const char *const lpuart_sels[] = { "pll3_80m", "osc", }; static const char *const semc_alt_sels[] = { "pll2_pfd2_396m", "pll3_pfd1_664_62m", }; static const char *const semc_sels[] = { "periph_sel", "semc_alt_sel", }; -static const char *const lcdif_sels[] = { "pll2_sys", "pll3_pfd3_454_74m", "pll5_video:", "pll2_pfd0_352m", "pll2_pfd1_594m", "pll3_pfd1_664_62m"}; +static const char *const lcdif_sels[] = { "pll2_sys", "pll3_pfd3_454_74m", "pll5_video", "pll2_pfd0_352m", "pll2_pfd1_594m", "pll3_pfd1_664_62m"};
static int imxrt1050_clk_probe(struct udevice *dev) {

On Wed, 26 Feb 2020 18:15:45 +0100 Giulio Benetti giulio.benetti@benettiengineering.com wrote:
"video:" must be "video", ":" is a typo.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com
drivers/clk/imx/clk-imxrt1050.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imxrt1050.c b/drivers/clk/imx/clk-imxrt1050.c index 44ca52c013..e33d426363 100644 --- a/drivers/clk/imx/clk-imxrt1050.c +++ b/drivers/clk/imx/clk-imxrt1050.c @@ -90,7 +90,7 @@ static const char *const usdhc_sels[] = { "pll2_pfd2_396m", "pll2_pfd0_352m", }; static const char *const lpuart_sels[] = { "pll3_80m", "osc", }; static const char *const semc_alt_sels[] = { "pll2_pfd2_396m", "pll3_pfd1_664_62m", }; static const char *const semc_sels[] = { "periph_sel", "semc_alt_sel", }; -static const char *const lcdif_sels[] = { "pll2_sys", "pll3_pfd3_454_74m", "pll5_video:", "pll2_pfd0_352m", "pll2_pfd1_594m", "pll3_pfd1_664_62m"}; +static const char *const lcdif_sels[] = { "pll2_sys", "pll3_pfd3_454_74m", "pll5_video", "pll2_pfd0_352m", "pll2_pfd1_594m", "pll3_pfd1_664_62m"}; static int imxrt1050_clk_probe(struct udevice *dev) {
Reviewed-by: Lukasz Majewski lukma@denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

mxsfb needs PLL5 as source, so let's setup it and set it as source for mxsfb(lcdif).
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/clk/imx/clk-imxrt1050.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imxrt1050.c b/drivers/clk/imx/clk-imxrt1050.c index e33d426363..2819ffb9ac 100644 --- a/drivers/clk/imx/clk-imxrt1050.c +++ b/drivers/clk/imx/clk-imxrt1050.c @@ -238,9 +238,9 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_dm(IMXRT1050_CLK_LCDIF, imx_clk_gate2("lcdif", "lcdif_podf", base + 0x70, 28));
-#ifdef CONFIG_SPL_BUILD struct clk *clk, *clk1;
+#ifdef CONFIG_SPL_BUILD /* bypass pll1 before setting its rate */ clk_get_by_id(IMXRT1050_CLK_PLL1_REF_SEL, &clk); clk_get_by_id(IMXRT1050_CLK_PLL1_BYPASS, &clk1); @@ -271,7 +271,18 @@ static int imxrt1050_clk_probe(struct udevice *dev)
clk_get_by_id(IMXRT1050_CLK_PLL3_BYPASS, &clk1); clk_set_parent(clk1, clk); +#else + /* Set PLL5 for LCDIF to its default 650Mhz */ + clk_get_by_id(IMXRT1050_CLK_PLL5_VIDEO, &clk); + clk_enable(clk); + clk_set_rate(clk, 650000000UL); + + clk_get_by_id(IMXRT1050_CLK_PLL5_BYPASS, &clk1); + clk_set_parent(clk1, clk);
+ /* Configure PLL5 as LCDIF source */ + clk_get_by_id(IMXRT1050_CLK_LCDIF_SEL, &clk1); + clk_set_parent(clk1, clk); #endif
return 0;

Hi Giulio,
On Wed, Feb 26, 2020 at 2:16 PM Giulio Benetti giulio.benetti@benettiengineering.com wrote:
mxsfb needs PLL5 as source, so let's setup it and set it as source for mxsfb(lcdif).
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com
drivers/clk/imx/clk-imxrt1050.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imxrt1050.c b/drivers/clk/imx/clk-imxrt1050.c index e33d426363..2819ffb9ac 100644 --- a/drivers/clk/imx/clk-imxrt1050.c +++ b/drivers/clk/imx/clk-imxrt1050.c @@ -238,9 +238,9 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_dm(IMXRT1050_CLK_LCDIF, imx_clk_gate2("lcdif", "lcdif_podf", base + 0x70, 28));
-#ifdef CONFIG_SPL_BUILD struct clk *clk, *clk1;
+#ifdef CONFIG_SPL_BUILD /* bypass pll1 before setting its rate */ clk_get_by_id(IMXRT1050_CLK_PLL1_REF_SEL, &clk); clk_get_by_id(IMXRT1050_CLK_PLL1_BYPASS, &clk1); @@ -271,7 +271,18 @@ static int imxrt1050_clk_probe(struct udevice *dev)
clk_get_by_id(IMXRT1050_CLK_PLL3_BYPASS, &clk1); clk_set_parent(clk1, clk);
+#else
/* Set PLL5 for LCDIF to its default 650Mhz */
clk_get_by_id(IMXRT1050_CLK_PLL5_VIDEO, &clk);
clk_enable(clk);
clk_set_rate(clk, 650000000UL);
clk_get_by_id(IMXRT1050_CLK_PLL5_BYPASS, &clk1);
clk_set_parent(clk1, clk);
/* Configure PLL5 as LCDIF source */
clk_get_by_id(IMXRT1050_CLK_LCDIF_SEL, &clk1);
clk_set_parent(clk1, clk);
This is more like a board design decision and IMHO should not be hardcoded as part of the clock driver.
Other users may want to use a different clock source for the eLCDIF driver.
Setting the clock parent in board device tree makes more sense.

Hi Fabio,
On 2/26/20 6:37 PM, Fabio Estevam wrote:
Hi Giulio,
On Wed, Feb 26, 2020 at 2:16 PM Giulio Benetti giulio.benetti@benettiengineering.com wrote:
mxsfb needs PLL5 as source, so let's setup it and set it as source for mxsfb(lcdif).
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com
drivers/clk/imx/clk-imxrt1050.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imxrt1050.c b/drivers/clk/imx/clk-imxrt1050.c index e33d426363..2819ffb9ac 100644 --- a/drivers/clk/imx/clk-imxrt1050.c +++ b/drivers/clk/imx/clk-imxrt1050.c @@ -238,9 +238,9 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_dm(IMXRT1050_CLK_LCDIF, imx_clk_gate2("lcdif", "lcdif_podf", base + 0x70, 28));
-#ifdef CONFIG_SPL_BUILD struct clk *clk, *clk1;
+#ifdef CONFIG_SPL_BUILD /* bypass pll1 before setting its rate */ clk_get_by_id(IMXRT1050_CLK_PLL1_REF_SEL, &clk); clk_get_by_id(IMXRT1050_CLK_PLL1_BYPASS, &clk1); @@ -271,7 +271,18 @@ static int imxrt1050_clk_probe(struct udevice *dev)
clk_get_by_id(IMXRT1050_CLK_PLL3_BYPASS, &clk1); clk_set_parent(clk1, clk);
+#else
/* Set PLL5 for LCDIF to its default 650Mhz */
clk_get_by_id(IMXRT1050_CLK_PLL5_VIDEO, &clk);
clk_enable(clk);
clk_set_rate(clk, 650000000UL);
clk_get_by_id(IMXRT1050_CLK_PLL5_BYPASS, &clk1);
clk_set_parent(clk1, clk);
/* Configure PLL5 as LCDIF source */
clk_get_by_id(IMXRT1050_CLK_LCDIF_SEL, &clk1);
clk_set_parent(clk1, clk);
This is more like a board design decision and IMHO should not be hardcoded as part of the clock driver.
Other users may want to use a different clock source for the eLCDIF driver.
Setting the clock parent in board device tree makes more sense.
Yes, it's a good idea. Doing this I've taken this[1] as example. So I don't know where in u-boot PLLs are initialized according to a dts file, can you please provide me an example? I will be happy to modify this according to that!
Thank you
[1]: https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/clk/imx/clk-imx8mm....
Best regards

Hi Giulio,
On Wed, Feb 26, 2020 at 2:54 PM Giulio Benetti giulio.benetti@benettiengineering.com wrote:
Yes, it's a good idea. Doing this I've taken this[1] as example. So I don't know where in u-boot PLLs are initialized according to a dts file, can you please provide me an example? I will be happy to modify this according to that!
In the kernel device trees we use the 'assigned-clocks' and 'assigned-clock-parents' properties to establish a clock parent relationship.
I suggest we follow the same approach in U-Boot.
Thanks

On 2/26/20 6:59 PM, Fabio Estevam wrote:
Hi Giulio,
On Wed, Feb 26, 2020 at 2:54 PM Giulio Benetti giulio.benetti@benettiengineering.com wrote:
Yes, it's a good idea. Doing this I've taken this[1] as example. So I don't know where in u-boot PLLs are initialized according to a dts file, can you please provide me an example? I will be happy to modify this according to that!
In the kernel device trees we use the 'assigned-clocks' and 'assigned-clock-parents' properties to establish a clock parent relationship.
I suggest we follow the same approach in U-Boot.
Oh, I've seen now, need to study it before, but now in my mind it's getting more clear how that works. But will this work even if shrinked CCF in u-boot can't set parent clocks(at least this is what I've understood)? I mean, basically here for LCDIF I see that only last divider get set for achieving pixel-clock, while all parents are get only to recalcute the "last divider parent clock".
Also, I can't understand, is it ok setting PLL5 to 650Mhz and un-bypass it? The problem is only about clk_set_parent() for LCDIF?
Because if a peripheral would set a PLL5 frequency and another peripheral use it as parent, then it would set it again.
Best regards

Hi Giulio,
On Wed, Feb 26, 2020 at 3:16 PM Giulio Benetti giulio.benetti@benettiengineering.com wrote:
Oh, I've seen now, need to study it before, but now in my mind it's getting more clear how that works. But will this work even if shrinked CCF in u-boot can't set parent clocks(at least this is what I've
I haven't checked whether 'assigned-clock-parents' works in U-Boot.
understood)? I mean, basically here for LCDIF I see that only last divider get set for achieving pixel-clock, while all parents are get only to recalcute the "last divider parent clock".
Also, I can't understand, is it ok setting PLL5 to 650Mhz and un-bypass it? The problem is only about clk_set_parent() for LCDIF?
The problem I saw was about hard coding the parent of LCDIF inside the clock driver.
Because if a peripheral would set a PLL5 frequency and another peripheral use it as parent, then it would set it again.
Yes, but if we leave the correct clock parent decision to be made in the board dts, we are safe.

On Wed, 26 Feb 2020 18:15:46 +0100 Giulio Benetti giulio.benetti@benettiengineering.com wrote:
mxsfb needs PLL5 as source, so let's setup it and set it as source for mxsfb(lcdif).
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com
drivers/clk/imx/clk-imxrt1050.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imxrt1050.c b/drivers/clk/imx/clk-imxrt1050.c index e33d426363..2819ffb9ac 100644 --- a/drivers/clk/imx/clk-imxrt1050.c +++ b/drivers/clk/imx/clk-imxrt1050.c @@ -238,9 +238,9 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_dm(IMXRT1050_CLK_LCDIF, imx_clk_gate2("lcdif", "lcdif_podf", base + 0x70, 28)); -#ifdef CONFIG_SPL_BUILD struct clk *clk, *clk1;
+#ifdef CONFIG_SPL_BUILD /* bypass pll1 before setting its rate */ clk_get_by_id(IMXRT1050_CLK_PLL1_REF_SEL, &clk); clk_get_by_id(IMXRT1050_CLK_PLL1_BYPASS, &clk1); @@ -271,7 +271,18 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_get_by_id(IMXRT1050_CLK_PLL3_BYPASS, &clk1); clk_set_parent(clk1, clk); +#else
/* Set PLL5 for LCDIF to its default 650Mhz */
clk_get_by_id(IMXRT1050_CLK_PLL5_VIDEO, &clk);
clk_enable(clk);
clk_set_rate(clk, 650000000UL);
clk_get_by_id(IMXRT1050_CLK_PLL5_BYPASS, &clk1);
clk_set_parent(clk1, clk);
/* Configure PLL5 as LCDIF source */
clk_get_by_id(IMXRT1050_CLK_LCDIF_SEL, &clk1);
clk_set_parent(clk1, clk);
#endif
return 0;
Reviewed-by: Lukasz Majewski lukma@denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Hi Lukasz,
On 3/8/20 9:27 PM, Lukasz Majewski wrote:
On Wed, 26 Feb 2020 18:15:46 +0100 Giulio Benetti giulio.benetti@benettiengineering.com wrote:
mxsfb needs PLL5 as source, so let's setup it and set it as source for mxsfb(lcdif).
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com
drivers/clk/imx/clk-imxrt1050.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imxrt1050.c b/drivers/clk/imx/clk-imxrt1050.c index e33d426363..2819ffb9ac 100644 --- a/drivers/clk/imx/clk-imxrt1050.c +++ b/drivers/clk/imx/clk-imxrt1050.c @@ -238,9 +238,9 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_dm(IMXRT1050_CLK_LCDIF, imx_clk_gate2("lcdif", "lcdif_podf", base + 0x70, 28)); -#ifdef CONFIG_SPL_BUILD struct clk *clk, *clk1;
+#ifdef CONFIG_SPL_BUILD /* bypass pll1 before setting its rate */ clk_get_by_id(IMXRT1050_CLK_PLL1_REF_SEL, &clk); clk_get_by_id(IMXRT1050_CLK_PLL1_BYPASS, &clk1); @@ -271,7 +271,18 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_get_by_id(IMXRT1050_CLK_PLL3_BYPASS, &clk1); clk_set_parent(clk1, clk); +#else
/* Set PLL5 for LCDIF to its default 650Mhz */
clk_get_by_id(IMXRT1050_CLK_PLL5_VIDEO, &clk);
clk_enable(clk);
clk_set_rate(clk, 650000000UL);
clk_get_by_id(IMXRT1050_CLK_PLL5_BYPASS, &clk1);
clk_set_parent(clk1, clk);
/* Configure PLL5 as LCDIF source */
clk_get_by_id(IMXRT1050_CLK_LCDIF_SEL, &clk1);
clk_set_parent(clk1, clk);
As pointed by Fabio, this ^^^ should be substituted with a using assigned-parent-clocks in dts instead of being hardcoded here. What do you think about it?
Thanks for reviewing and best regards

On Sun, 8 Mar 2020 22:05:42 +0100 Giulio Benetti giulio.benetti@benettiengineering.com wrote:
Hi Lukasz,
On 3/8/20 9:27 PM, Lukasz Majewski wrote:
On Wed, 26 Feb 2020 18:15:46 +0100 Giulio Benetti giulio.benetti@benettiengineering.com wrote:
mxsfb needs PLL5 as source, so let's setup it and set it as source for mxsfb(lcdif).
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/clk/imx/clk-imxrt1050.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imxrt1050.c b/drivers/clk/imx/clk-imxrt1050.c index e33d426363..2819ffb9ac 100644 --- a/drivers/clk/imx/clk-imxrt1050.c +++ b/drivers/clk/imx/clk-imxrt1050.c @@ -238,9 +238,9 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_dm(IMXRT1050_CLK_LCDIF, imx_clk_gate2("lcdif", "lcdif_podf", base + 0x70, 28)); -#ifdef CONFIG_SPL_BUILD struct clk *clk, *clk1;
+#ifdef CONFIG_SPL_BUILD /* bypass pll1 before setting its rate */ clk_get_by_id(IMXRT1050_CLK_PLL1_REF_SEL, &clk); clk_get_by_id(IMXRT1050_CLK_PLL1_BYPASS, &clk1); @@ -271,7 +271,18 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_get_by_id(IMXRT1050_CLK_PLL3_BYPASS, &clk1); clk_set_parent(clk1, clk); +#else
/* Set PLL5 for LCDIF to its default 650Mhz */
clk_get_by_id(IMXRT1050_CLK_PLL5_VIDEO, &clk);
clk_enable(clk);
clk_set_rate(clk, 650000000UL);
clk_get_by_id(IMXRT1050_CLK_PLL5_BYPASS, &clk1);
clk_set_parent(clk1, clk);
/* Configure PLL5 as LCDIF source */
clk_get_by_id(IMXRT1050_CLK_LCDIF_SEL, &clk1);
clk_set_parent(clk1, clk);
As pointed by Fabio, this ^^^ should be substituted with a using assigned-parent-clocks in dts instead of being hardcoded here.
Upss.. Apparently I've missed the conversation. Thanks for pointing this out.
What do you think about it?
If it is relatively easy to do then I'm for it.
Thanks for reviewing and best regards
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Hi Lukasz, Fabio,
On 3/9/20 10:11 AM, Lukasz Majewski wrote:
On Sun, 8 Mar 2020 22:05:42 +0100 Giulio Benetti giulio.benetti@benettiengineering.com wrote:
Hi Lukasz,
On 3/8/20 9:27 PM, Lukasz Majewski wrote:
On Wed, 26 Feb 2020 18:15:46 +0100 Giulio Benetti giulio.benetti@benettiengineering.com wrote:
mxsfb needs PLL5 as source, so let's setup it and set it as source for mxsfb(lcdif).
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/clk/imx/clk-imxrt1050.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imxrt1050.c b/drivers/clk/imx/clk-imxrt1050.c index e33d426363..2819ffb9ac 100644 --- a/drivers/clk/imx/clk-imxrt1050.c +++ b/drivers/clk/imx/clk-imxrt1050.c @@ -238,9 +238,9 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_dm(IMXRT1050_CLK_LCDIF, imx_clk_gate2("lcdif", "lcdif_podf", base + 0x70, 28)); -#ifdef CONFIG_SPL_BUILD struct clk *clk, *clk1;
+#ifdef CONFIG_SPL_BUILD /* bypass pll1 before setting its rate */ clk_get_by_id(IMXRT1050_CLK_PLL1_REF_SEL, &clk); clk_get_by_id(IMXRT1050_CLK_PLL1_BYPASS, &clk1); @@ -271,7 +271,18 @@ static int imxrt1050_clk_probe(struct udevice *dev) clk_get_by_id(IMXRT1050_CLK_PLL3_BYPASS, &clk1); clk_set_parent(clk1, clk); +#else
/* Set PLL5 for LCDIF to its default 650Mhz */
clk_get_by_id(IMXRT1050_CLK_PLL5_VIDEO, &clk);
clk_enable(clk);
clk_set_rate(clk, 650000000UL);
clk_get_by_id(IMXRT1050_CLK_PLL5_BYPASS, &clk1);
clk_set_parent(clk1, clk);
/* Configure PLL5 as LCDIF source */
clk_get_by_id(IMXRT1050_CLK_LCDIF_SEL, &clk1);
clk_set_parent(clk1, clk);
As pointed by Fabio, this ^^^ should be substituted with a using assigned-parent-clocks in dts instead of being hardcoded here.
Upss.. Apparently I've missed the conversation. Thanks for pointing this out.
What do you think about it?
If it is relatively easy to do then I'm for it.
Yes, I've done it.
I'm going to send v2 series soon.
Best regards

This function converts from "struct ctf_res_modes" to "struct display_timing".
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/video/videomodes.c | 29 +++++++++++++++++++++++++++++ drivers/video/videomodes.h | 3 +++ 2 files changed, 32 insertions(+)
diff --git a/drivers/video/videomodes.c b/drivers/video/videomodes.c index ac25b45f81..89003eea72 100644 --- a/drivers/video/videomodes.c +++ b/drivers/video/videomodes.c @@ -444,3 +444,32 @@ int video_edid_dtd_to_ctfb_res_modes(struct edid_detailed_timing *t,
return 0; } + +void video_ctfb_mode_to_display_timing(const struct ctfb_res_modes *mode, + struct display_timing *timing) +{ + timing->pixelclock.typ = mode->pixclock_khz * 1000; + + timing->hactive.typ = mode->xres; + timing->hfront_porch.typ = mode->right_margin; + timing->hback_porch.typ = mode->left_margin; + timing->hsync_len.typ = mode->hsync_len; + + timing->vactive.typ = mode->yres; + timing->vfront_porch.typ = mode->lower_margin; + timing->vback_porch.typ = mode->upper_margin; + timing->vsync_len.typ = mode->vsync_len; + + timing->flags = 0; + + if (mode->sync & FB_SYNC_HOR_HIGH_ACT) + timing->flags |= DISPLAY_FLAGS_HSYNC_HIGH; + else + timing->flags |= DISPLAY_FLAGS_HSYNC_LOW; + if (mode->sync & FB_SYNC_VERT_HIGH_ACT) + timing->flags |= DISPLAY_FLAGS_VSYNC_HIGH; + else + timing->flags |= DISPLAY_FLAGS_VSYNC_LOW; + if (mode->vmode == FB_VMODE_INTERLACED) + timing->flags |= DISPLAY_FLAGS_INTERLACED; +} diff --git a/drivers/video/videomodes.h b/drivers/video/videomodes.h index 29a3db4ae3..6713f96d19 100644 --- a/drivers/video/videomodes.h +++ b/drivers/video/videomodes.h @@ -92,3 +92,6 @@ int video_get_option_int(const char *options, const char *name, int def);
int video_edid_dtd_to_ctfb_res_modes(struct edid_detailed_timing *t, struct ctfb_res_modes *mode); + +void video_ctfb_mode_to_display_timing(const struct ctfb_res_modes *mode, + struct display_timing *timing);

Since video_ctfb_mode_to_display_timing() has been implemented by moving sunxi_ctfb_mode_to_display_timing() to video_modes.c and it's meant to be used by other video subsystem, let's use it instead of local sunxi_ctfb_mode_to_display_timing().
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/video/sunxi/sunxi_display.c | 33 ++--------------------------- 1 file changed, 2 insertions(+), 31 deletions(-)
diff --git a/drivers/video/sunxi/sunxi_display.c b/drivers/video/sunxi/sunxi_display.c index 31f0aa7ddc..a6a62c83ef 100644 --- a/drivers/video/sunxi/sunxi_display.c +++ b/drivers/video/sunxi/sunxi_display.c @@ -615,35 +615,6 @@ static void sunxi_lcdc_backlight_enable(void) gpio_direction_output(pin, PWM_ON); }
-static void sunxi_ctfb_mode_to_display_timing(const struct ctfb_res_modes *mode, - struct display_timing *timing) -{ - timing->pixelclock.typ = mode->pixclock_khz * 1000; - - timing->hactive.typ = mode->xres; - timing->hfront_porch.typ = mode->right_margin; - timing->hback_porch.typ = mode->left_margin; - timing->hsync_len.typ = mode->hsync_len; - - timing->vactive.typ = mode->yres; - timing->vfront_porch.typ = mode->lower_margin; - timing->vback_porch.typ = mode->upper_margin; - timing->vsync_len.typ = mode->vsync_len; - - timing->flags = 0; - - if (mode->sync & FB_SYNC_HOR_HIGH_ACT) - timing->flags |= DISPLAY_FLAGS_HSYNC_HIGH; - else - timing->flags |= DISPLAY_FLAGS_HSYNC_LOW; - if (mode->sync & FB_SYNC_VERT_HIGH_ACT) - timing->flags |= DISPLAY_FLAGS_VSYNC_HIGH; - else - timing->flags |= DISPLAY_FLAGS_VSYNC_LOW; - if (mode->vmode == FB_VMODE_INTERLACED) - timing->flags |= DISPLAY_FLAGS_INTERLACED; -} - static void sunxi_lcdc_tcon0_mode_set(const struct ctfb_res_modes *mode, bool for_ext_vga_dac) { @@ -673,7 +644,7 @@ static void sunxi_lcdc_tcon0_mode_set(const struct ctfb_res_modes *mode, lcdc_pll_set(ccm, 0, mode->pixclock_khz, &clk_div, &clk_double, sunxi_is_composite());
- sunxi_ctfb_mode_to_display_timing(mode, &timing); + video_ctfb_mode_to_display_timing(mode, &timing); lcdc_tcon0_mode_set(lcdc, &timing, clk_div, for_ext_vga_dac, sunxi_display.depth, CONFIG_VIDEO_LCD_DCLK_PHASE); } @@ -689,7 +660,7 @@ static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode, (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; struct display_timing timing;
- sunxi_ctfb_mode_to_display_timing(mode, &timing); + video_ctfb_mode_to_display_timing(mode, &timing); lcdc_tcon1_mode_set(lcdc, &timing, use_portd_hvsync, sunxi_is_composite());

Allow using DM CLK instead of mxs_set_lcdclk() so we can avoid to implement a special function to set lcd clock on i.MXRT.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/video/mxsfb.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c index 585af3d571..f21f8247d9 100644 --- a/drivers/video/mxsfb.c +++ b/drivers/video/mxsfb.c @@ -5,6 +5,7 @@ * Copyright (C) 2011-2013 Marek Vasut marex@denx.de */ #include <common.h> +#include <clk.h> #include <dm.h> #include <env.h> #include <dm/device_compat.h> @@ -52,14 +53,32 @@ __weak void mxsfb_system_setup(void) * le:89,ri:164,up:23,lo:10,hs:10,vs:10,sync:0,vmode:0 */
-static void mxs_lcd_init(u32 fb_addr, struct ctfb_res_modes *mode, int bpp) +static void mxs_lcd_init(struct udevice *dev, u32 fb_addr, + struct ctfb_res_modes *mode, int bpp) { struct mxs_lcdif_regs *regs = (struct mxs_lcdif_regs *)MXS_LCDIF_BASE; uint32_t word_len = 0, bus_width = 0; uint8_t valid_data = 0;
+#if CONFIG_IS_ENABLED(CLK) + struct clk per_clk; + int ret; + + ret = clk_get_by_name(dev, "per", &per_clk); + if (ret) { + dev_err(dev, "Failed to get mxs clk: %d\n", ret); + return; + } + + ret = clk_set_rate(&per_clk, PS2KHZ(mode->pixclock) * 1000); + if (ret < 0) { + dev_err(dev, "Failed to set mxs clk: %d\n", ret); + return; + } +#else /* Kick in the LCDIF clock */ mxs_set_lcdclk(MXS_LCDIF_BASE, PS2KHZ(mode->pixclock)); +#endif
/* Restart the LCDIF block */ mxs_reset_block(®s->hw_lcdif_ctrl_reg); @@ -135,10 +154,11 @@ static void mxs_lcd_init(u32 fb_addr, struct ctfb_res_modes *mode, int bpp) writel(LCDIF_CTRL_RUN, ®s->hw_lcdif_ctrl_set); }
-static int mxs_probe_common(struct ctfb_res_modes *mode, int bpp, u32 fb) +static int mxs_probe_common(struct udevice *dev, struct ctfb_res_modes *mode, + int bpp, u32 fb) { /* Start framebuffer */ - mxs_lcd_init(fb, mode, bpp); + mxs_lcd_init(dev, fb, mode, bpp);
#ifdef CONFIG_VIDEO_MXS_MODE_SYSTEM /* @@ -260,7 +280,7 @@ void *video_hw_init(void)
printf("%s\n", panel.modeIdent);
- ret = mxs_probe_common(&mode, bpp, (u32)fb); + ret = mxs_probe_common(NULL, &mode, bpp, (u32)fb); if (ret) goto dealloc_fb;
@@ -337,7 +357,7 @@ static int mxs_video_probe(struct udevice *dev) mode.vsync_len = timings.vsync_len.typ; mode.pixclock = HZ2PS(timings.pixelclock.typ);
- ret = mxs_probe_common(&mode, bpp, plat->base); + ret = mxs_probe_common(dev, &mode, bpp, plat->base); if (ret) return ret;

Add support for i.MXRT by adding CONFIG_IMXRT in register structure and adding .compatible = "fsl,imxrt-lcdif".
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- arch/arm/include/asm/arch-imxrt/imx-regs.h | 6 ++++++ arch/arm/include/asm/mach-imx/regs-lcdif.h | 6 +++--- drivers/video/mxsfb.c | 1 + 3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/arch-imxrt/imx-regs.h b/arch/arm/include/asm/arch-imxrt/imx-regs.h index 4f1d439f6f..44c95dcd11 100644 --- a/arch/arm/include/asm/arch-imxrt/imx-regs.h +++ b/arch/arm/include/asm/arch-imxrt/imx-regs.h @@ -17,4 +17,10 @@
#define ANATOP_BASE_ADDR 0x400d8000
+#define MXS_LCDIF_BASE 0x402b8000 + +#if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__)) +#include <asm/mach-imx/regs-lcdif.h> +#endif + #endif /* __ASM_ARCH_IMX_REGS_H__ */ diff --git a/arch/arm/include/asm/mach-imx/regs-lcdif.h b/arch/arm/include/asm/mach-imx/regs-lcdif.h index b4c430a35c..5874638796 100644 --- a/arch/arm/include/asm/mach-imx/regs-lcdif.h +++ b/arch/arm/include/asm/mach-imx/regs-lcdif.h @@ -22,7 +22,7 @@ struct mxs_lcdif_regs { defined(CONFIG_MX6SL) || defined(CONFIG_MX6SLL) || \ defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL) || \ defined(CONFIG_MX7) || defined(CONFIG_MX7ULP) || \ - defined(CONFIG_IMX8M) + defined(CONFIG_IMX8M) || defined(CONFIG_IMXRT) mxs_reg_32(hw_lcdif_ctrl2) /* 0x20 */ #endif mxs_reg_32(hw_lcdif_transfer_count) /* 0x20/0x30 */ @@ -49,7 +49,7 @@ struct mxs_lcdif_regs { mxs_reg_32(hw_lcdif_csc_coeffctrl2) /* 0x130 */ mxs_reg_32(hw_lcdif_csc_coeffctrl3) /* 0x140 */ mxs_reg_32(hw_lcdif_csc_coeffctrl4) /* 0x150 */ - mxs_reg_32(hw_lcdif_csc_offset) /* 0x160 */ + mxs_reg_32(hw_lcdif_csc_offset) /* 0x160 */ mxs_reg_32(hw_lcdif_csc_limit) /* 0x170 */
#if defined(CONFIG_MX23) @@ -61,7 +61,7 @@ struct mxs_lcdif_regs { defined(CONFIG_MX6SL) || defined(CONFIG_MX6SLL) || \ defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL) || \ defined(CONFIG_MX7) || defined(CONFIG_MX7ULP) || \ - defined(CONFIG_IMX8M) + defined(CONFIG_IMX8M) || defined(CONFIG_IMXRT) mxs_reg_32(hw_lcdif_crc_stat) /* 0x1a0 */ #endif mxs_reg_32(hw_lcdif_lcdif_stat) /* 0x1d0/0x1b0 */ diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c index f21f8247d9..6826ba3d1b 100644 --- a/drivers/video/mxsfb.c +++ b/drivers/video/mxsfb.c @@ -440,6 +440,7 @@ static const struct udevice_id mxs_video_ids[] = { { .compatible = "fsl,imx23-lcdif" }, { .compatible = "fsl,imx28-lcdif" }, { .compatible = "fsl,imx7ulp-lcdif" }, + { .compatible = "fsl,imxrt-lcdif" }, { /* sentinel */ } };

struct display_timings provides more informations such clock and DE polarity, so let's refactor the code to use struct display_timings instead of struct ctfb_res_modes, so we'll become able to get clock and DE polarity settings and set register according to them in the next patch.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/video/mxsfb.c | 54 ++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 31 deletions(-)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c index 6826ba3d1b..cdd6dfaced 100644 --- a/drivers/video/mxsfb.c +++ b/drivers/video/mxsfb.c @@ -54,7 +54,7 @@ __weak void mxsfb_system_setup(void) */
static void mxs_lcd_init(struct udevice *dev, u32 fb_addr, - struct ctfb_res_modes *mode, int bpp) + struct display_timing *timings, int bpp) { struct mxs_lcdif_regs *regs = (struct mxs_lcdif_regs *)MXS_LCDIF_BASE; uint32_t word_len = 0, bus_width = 0; @@ -70,14 +70,14 @@ static void mxs_lcd_init(struct udevice *dev, u32 fb_addr, return; }
- ret = clk_set_rate(&per_clk, PS2KHZ(mode->pixclock) * 1000); + ret = clk_set_rate(&per_clk, timings->pixelclock.typ); if (ret < 0) { dev_err(dev, "Failed to set mxs clk: %d\n", ret); return; } #else /* Kick in the LCDIF clock */ - mxs_set_lcdclk(MXS_LCDIF_BASE, PS2KHZ(mode->pixclock)); + mxs_set_lcdclk(MXS_LCDIF_BASE, timings->pixelclock.typ / 1000); #endif
/* Restart the LCDIF block */ @@ -115,25 +115,25 @@ static void mxs_lcd_init(struct udevice *dev, u32 fb_addr,
mxsfb_system_setup();
- writel((mode->yres << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | mode->xres, - ®s->hw_lcdif_transfer_count); + writel((timings->vactive.typ << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | + timings->hactive.typ, ®s->hw_lcdif_transfer_count);
writel(LCDIF_VDCTRL0_ENABLE_PRESENT | LCDIF_VDCTRL0_ENABLE_POL | LCDIF_VDCTRL0_VSYNC_PERIOD_UNIT | LCDIF_VDCTRL0_VSYNC_PULSE_WIDTH_UNIT | - mode->vsync_len, ®s->hw_lcdif_vdctrl0); - writel(mode->upper_margin + mode->lower_margin + - mode->vsync_len + mode->yres, + timings->vsync_len.typ, ®s->hw_lcdif_vdctrl0); + writel(timings->vback_porch.typ + timings->vfront_porch.typ + + timings->vsync_len.typ + timings->vactive.typ, ®s->hw_lcdif_vdctrl1); - writel((mode->hsync_len << LCDIF_VDCTRL2_HSYNC_PULSE_WIDTH_OFFSET) | - (mode->left_margin + mode->right_margin + - mode->hsync_len + mode->xres), + writel((timings->hsync_len.typ << LCDIF_VDCTRL2_HSYNC_PULSE_WIDTH_OFFSET) | + (timings->hback_porch.typ + timings->hfront_porch.typ + + timings->hsync_len.typ + timings->hactive.typ), ®s->hw_lcdif_vdctrl2); - writel(((mode->left_margin + mode->hsync_len) << + writel(((timings->hback_porch.typ + timings->hsync_len.typ) << LCDIF_VDCTRL3_HORIZONTAL_WAIT_CNT_OFFSET) | - (mode->upper_margin + mode->vsync_len), + (timings->vback_porch.typ + timings->vsync_len.typ), ®s->hw_lcdif_vdctrl3); - writel((0 << LCDIF_VDCTRL4_DOTCLK_DLY_SEL_OFFSET) | mode->xres, + writel((0 << LCDIF_VDCTRL4_DOTCLK_DLY_SEL_OFFSET) | timings->hactive.typ, ®s->hw_lcdif_vdctrl4);
writel(fb_addr, ®s->hw_lcdif_cur_buf); @@ -154,11 +154,11 @@ static void mxs_lcd_init(struct udevice *dev, u32 fb_addr, writel(LCDIF_CTRL_RUN, ®s->hw_lcdif_ctrl_set); }
-static int mxs_probe_common(struct udevice *dev, struct ctfb_res_modes *mode, +static int mxs_probe_common(struct udevice *dev, struct display_timing *timings, int bpp, u32 fb) { /* Start framebuffer */ - mxs_lcd_init(dev, fb, mode, bpp); + mxs_lcd_init(dev, fb, timings, bpp);
#ifdef CONFIG_VIDEO_MXS_MODE_SYSTEM /* @@ -224,6 +224,7 @@ void *video_hw_init(void) char *penv; void *fb = NULL; struct ctfb_res_modes mode; + struct display_timing timings;
puts("Video: ");
@@ -280,7 +281,9 @@ void *video_hw_init(void)
printf("%s\n", panel.modeIdent);
- ret = mxs_probe_common(NULL, &mode, bpp, (u32)fb); + video_ctfb_mode_to_display_timing(&mode, &timings); + + ret = mxs_probe_common(NULL, &timings, bpp, (u32)fb); if (ret) goto dealloc_fb;
@@ -334,7 +337,6 @@ static int mxs_video_probe(struct udevice *dev) struct video_uc_platdata *plat = dev_get_uclass_platdata(dev); struct video_priv *uc_priv = dev_get_uclass_priv(dev);
- struct ctfb_res_modes mode; struct display_timing timings; u32 bpp = 0; u32 fb_start, fb_end; @@ -347,17 +349,7 @@ static int mxs_video_probe(struct udevice *dev) if (ret) return ret;
- mode.xres = timings.hactive.typ; - mode.yres = timings.vactive.typ; - mode.left_margin = timings.hback_porch.typ; - mode.right_margin = timings.hfront_porch.typ; - mode.upper_margin = timings.vback_porch.typ; - mode.lower_margin = timings.vfront_porch.typ; - mode.hsync_len = timings.hsync_len.typ; - mode.vsync_len = timings.vsync_len.typ; - mode.pixclock = HZ2PS(timings.pixelclock.typ); - - ret = mxs_probe_common(dev, &mode, bpp, plat->base); + ret = mxs_probe_common(dev, &timings, bpp, plat->base); if (ret) return ret;
@@ -378,8 +370,8 @@ static int mxs_video_probe(struct udevice *dev) return -EINVAL; }
- uc_priv->xsize = mode.xres; - uc_priv->ysize = mode.yres; + uc_priv->xsize = timings.hactive.typ; + uc_priv->ysize = timings.vactive.typ;
/* Enable dcache for the frame buffer */ fb_start = plat->base & ~(MMU_SECTION_SIZE - 1);

HSYNC signal can now be flipped according to display_flags bitmaks by writing its bitmask on vdctrl0 register.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/video/mxsfb.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c index cdd6dfaced..9912cf3d82 100644 --- a/drivers/video/mxsfb.c +++ b/drivers/video/mxsfb.c @@ -57,8 +57,10 @@ static void mxs_lcd_init(struct udevice *dev, u32 fb_addr, struct display_timing *timings, int bpp) { struct mxs_lcdif_regs *regs = (struct mxs_lcdif_regs *)MXS_LCDIF_BASE; + const enum display_flags flags = timings->flags; uint32_t word_len = 0, bus_width = 0; uint8_t valid_data = 0; + uint32_t vdctrl0;
#if CONFIG_IS_ENABLED(CLK) struct clk per_clk; @@ -118,10 +120,14 @@ static void mxs_lcd_init(struct udevice *dev, u32 fb_addr, writel((timings->vactive.typ << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | timings->hactive.typ, ®s->hw_lcdif_transfer_count);
- writel(LCDIF_VDCTRL0_ENABLE_PRESENT | LCDIF_VDCTRL0_ENABLE_POL | - LCDIF_VDCTRL0_VSYNC_PERIOD_UNIT | - LCDIF_VDCTRL0_VSYNC_PULSE_WIDTH_UNIT | - timings->vsync_len.typ, ®s->hw_lcdif_vdctrl0); + vdctrl0 = LCDIF_VDCTRL0_ENABLE_PRESENT | LCDIF_VDCTRL0_ENABLE_POL | + LCDIF_VDCTRL0_VSYNC_PERIOD_UNIT | + LCDIF_VDCTRL0_VSYNC_PULSE_WIDTH_UNIT | + timings->vsync_len.typ; + + if(flags & DISPLAY_FLAGS_HSYNC_HIGH) + vdctrl0 |= LCDIF_VDCTRL0_HSYNC_POL; + writel(vdctrl0, ®s->hw_lcdif_vdctrl0); writel(timings->vback_porch.typ + timings->vfront_porch.typ + timings->vsync_len.typ + timings->vactive.typ, ®s->hw_lcdif_vdctrl1);

VSYNC signal can now be flipped by writing its bitmask on vdctrl0 register.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/video/mxsfb.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c index 9912cf3d82..4d33e24e1a 100644 --- a/drivers/video/mxsfb.c +++ b/drivers/video/mxsfb.c @@ -127,6 +127,8 @@ static void mxs_lcd_init(struct udevice *dev, u32 fb_addr,
if(flags & DISPLAY_FLAGS_HSYNC_HIGH) vdctrl0 |= LCDIF_VDCTRL0_HSYNC_POL; + if(flags & DISPLAY_FLAGS_VSYNC_HIGH) + vdctrl0 |= LCDIF_VDCTRL0_VSYNC_POL; writel(vdctrl0, ®s->hw_lcdif_vdctrl0); writel(timings->vback_porch.typ + timings->vfront_porch.typ + timings->vsync_len.typ + timings->vactive.typ,

DOTCLK signal can now be flipped by writing its bitmask on vdctrl0 register.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/video/mxsfb.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c index 4d33e24e1a..648e1c22fe 100644 --- a/drivers/video/mxsfb.c +++ b/drivers/video/mxsfb.c @@ -129,6 +129,8 @@ static void mxs_lcd_init(struct udevice *dev, u32 fb_addr, vdctrl0 |= LCDIF_VDCTRL0_HSYNC_POL; if(flags & DISPLAY_FLAGS_VSYNC_HIGH) vdctrl0 |= LCDIF_VDCTRL0_VSYNC_POL; + if(flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) + vdctrl0 |= LCDIF_VDCTRL0_DOTCLK_POL; writel(vdctrl0, ®s->hw_lcdif_vdctrl0); writel(timings->vback_porch.typ + timings->vfront_porch.typ + timings->vsync_len.typ + timings->vactive.typ,

ENABLE signal can now be flipped by writing its bitmask on vdctrl0 register.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/video/mxsfb.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c index 648e1c22fe..8a5a61c9fb 100644 --- a/drivers/video/mxsfb.c +++ b/drivers/video/mxsfb.c @@ -131,6 +131,9 @@ static void mxs_lcd_init(struct udevice *dev, u32 fb_addr, vdctrl0 |= LCDIF_VDCTRL0_VSYNC_POL; if(flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) vdctrl0 |= LCDIF_VDCTRL0_DOTCLK_POL; + if(flags & DISPLAY_FLAGS_DE_HIGH) + vdctrl0 |= LCDIF_VDCTRL0_ENABLE_POL; + writel(vdctrl0, ®s->hw_lcdif_vdctrl0); writel(timings->vback_porch.typ + timings->vfront_porch.typ + timings->vsync_len.typ + timings->vactive.typ,

i.MXRT1050 provides mxsfb compatible lcd controller, so let's enable video mxsfb driver with 16bpp depth if CONFIG_DM_VIDEO is selected since board has 16bpp only connection.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- include/configs/imxrt1050-evk.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/include/configs/imxrt1050-evk.h b/include/configs/imxrt1050-evk.h index cdec657fb0..3a6b972d9a 100644 --- a/include/configs/imxrt1050-evk.h +++ b/include/configs/imxrt1050-evk.h @@ -30,6 +30,21 @@
#define CONFIG_SYS_MMC_ENV_DEV 0 /* USDHC1 */
+#ifdef CONFIG_DM_VIDEO +#define CONFIG_VIDEO_MXS +#define CONFIG_VIDEO_LOGO +#define CONFIG_SPLASH_SCREEN +#define CONFIG_SPLASH_SCREEN_ALIGN +#define CONFIG_BMP_16BPP +#define CONFIG_VIDEO_BMP_RLE8 +#define CONFIG_VIDEO_BMP_LOGO + +#define CONFIG_EXTRA_ENV_SETTINGS \ + "stdin=serial\0" \ + "stdout=serial,vidconsole\0" \ + "stderr=serial,vidconsole\0" +#endif + /* * Configuration of the external SDRAM memory */

Add lcdif node to SoC.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- arch/arm/dts/imxrt1050.dtsi | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/arch/arm/dts/imxrt1050.dtsi b/arch/arm/dts/imxrt1050.dtsi index b1d98e6feb..0123f4788c 100644 --- a/arch/arm/dts/imxrt1050.dtsi +++ b/arch/arm/dts/imxrt1050.dtsi @@ -13,6 +13,7 @@
/ { aliases { + display0 = &lcdif; gpio0 = &gpio1; gpio1 = &gpio2; gpio2 = &gpio3; @@ -142,5 +143,14 @@ interrupt-controller; #interrupt-cells = <2>; }; + + lcdif: lcdif@402b8000 { + compatible = "fsl,imxrt-lcdif"; + reg = <0x402b8000 0x10000>; + interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clks IMXRT1050_CLK_LCDIF>; + clock-names = "per"; + status = "disabled"; + }; }; };

Linux doesn't provide skeleton.dtsi file so let's remove its include and provide #address-cells/size-cells = <1> that were defined in skeleton.dtsi before.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- arch/arm/dts/imxrt1050.dtsi | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/imxrt1050.dtsi b/arch/arm/dts/imxrt1050.dtsi index 0123f4788c..7cfe5f5c95 100644 --- a/arch/arm/dts/imxrt1050.dtsi +++ b/arch/arm/dts/imxrt1050.dtsi @@ -4,7 +4,6 @@ * Author(s): Giulio Benetti giulio.benetti@benettiengineering.com */
-#include "skeleton.dtsi" #include "armv7-m.dtsi" #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/clock/imxrt1050-clock.h> @@ -12,6 +11,9 @@ #include <dt-bindings/memory/imxrt-sdram.h>
/ { + #address-cells = <1>; + #size-cells = <1>; + aliases { display0 = &lcdif; gpio0 = &gpio1;

Forgot to mention that this patchset needs this patch before: https://patchwork.ozlabs.org/patch/1232248/

Hi All,
On 2/26/20 6:15 PM, Giulio Benetti wrote:
This patchset add support for LCDIF on i.MXRT1050 evk. This requires PLL5 to be setup, mxsfb needs to use display_timing to retrieve if Lcd has inverted PIXCLOCK from dts.
With this patchset applied we temporary loose DCache support until it will get implemented, since a function in mxsfb.c is needed for setting cache behaviour. Anyway this way Lcd will show the console same way as serial does.
Also I've moved private sunxi_ctfb_mode_to_display_timing() to videomodes since I need it for mxfsb.c too, then having a unified function to convert from ctfb_mode to display_timing.
Giulio Benetti (18): clk: imx: pllv3: add enable_bit clk: imx: imxrt1050-clk: fix typo in clock name "video:" clk: imx: clk-imxrt1050: setup PLL5 for video in non-SPL videomodes: add helper function to convert from ctfb to display_timing sunxi: display: use common video_ctfb_mode_to_display_timing() video: mxsfb: add support for DM CLK video: mxsfb: add support for i.MXRT video: mxsfb: refactor for using display_timings video: mxsfb: enable setting HSYNC negative polarity video: mxsfb: enable setting VSYNC negative polarity video: mxsfb: enable setting PIXDATA on negative edge video: mxsfb: enable setting ENABLE negative polarity
kindly ping for all "video: " and "sunxi: " patches. I've already fixed what Fabio and Lukasz pointed about clock-parents, sowhen can I send v2-series? Does it look ok the rest?
Thanks in advance
Best regards
participants (3)
-
Fabio Estevam
-
Giulio Benetti
-
Lukasz Majewski