imx8mm eLCDIF clock

Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the following code to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this?
Thanks, Tommmaso

On Thu, Apr 21, 2022 at 08:48:05AM +0200, Tommaso Merciai wrote:
+ Fabio + Tim + Michael + Marek + Adam
Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the clk_enable to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this? I'm missing somethings?
Thanks, Tommmaso -- Tommaso Merciai Embedded Linux Engineer tommaso.merciai@amarulasolutions.com __________________________________
Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@amarulasolutions.com www.amarulasolutions.com

On Thu, Apr 21, 2022 at 5:29 AM Tommaso Merciai tommaso.merciai@amarulasolutions.com wrote:
On Thu, Apr 21, 2022 at 08:48:05AM +0200, Tommaso Merciai wrote:
- Fabio
- Tim
- Michael
- Marek
- Adam
Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the clk_enable to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this? I'm missing somethings?
I think the LCD driver should request the clock and clock rate based on settings the device tree. However, I think the bigger issues is that you might run into issues with the lack of a disp-blkctrl driver. Marek enable the GPC driver fairly recently, but the blkctrl driver will be needed to enable the LCD and DSI portions or the system may hang.
adam
Thanks, Tommmaso -- Tommaso Merciai Embedded Linux Engineer tommaso.merciai@amarulasolutions.com __________________________________
Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@amarulasolutions.com www.amarulasolutions.com
-- Tommaso Merciai Embedded Linux Engineer tommaso.merciai@amarulasolutions.com __________________________________
Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@amarulasolutions.com www.amarulasolutions.com

On 4/21/22 13:14, Adam Ford wrote:
On Thu, Apr 21, 2022 at 5:29 AM Tommaso Merciai tommaso.merciai@amarulasolutions.com wrote:
On Thu, Apr 21, 2022 at 08:48:05AM +0200, Tommaso Merciai wrote:
- Fabio
- Tim
- Michael
- Marek
- Adam
Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the clk_enable to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this? I'm missing somethings?
I think the LCD driver should request the clock and clock rate based on settings the device tree. However, I think the bigger issues is that you might run into issues with the lack of a disp-blkctrl driver. Marek enable the GPC driver fairly recently, but the blkctrl driver will be needed to enable the LCD and DSI portions or the system may hang.
Just boot quickly and init the graphics pipeline in Linux ?

On Thu, Apr 21, 2022 at 01:20:58PM +0200, Marek Vasut wrote:
On 4/21/22 13:14, Adam Ford wrote:
On Thu, Apr 21, 2022 at 5:29 AM Tommaso Merciai tommaso.merciai@amarulasolutions.com wrote:
On Thu, Apr 21, 2022 at 08:48:05AM +0200, Tommaso Merciai wrote:
- Fabio
- Tim
- Michael
- Marek
- Adam
Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the clk_enable to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this? I'm missing somethings?
I think the LCD driver should request the clock and clock rate based on settings the device tree. However, I think the bigger issues is that you might run into issues with the lack of a disp-blkctrl driver. Marek enable the GPC driver fairly recently, but the blkctrl driver will be needed to enable the LCD and DSI portions or the system may hang.
Just boot quickly and init the graphics pipeline in Linux ?
Hi Marek, Thanks for your feedback. You think make no sense to enable support for the graphics pipeline at U-Boot level? Some customers want this feature for that I think is better to have support for that. What do you think about?
Thanks, Tommaso

On 4/21/22 13:54, Tommaso Merciai wrote:
On Thu, Apr 21, 2022 at 01:20:58PM +0200, Marek Vasut wrote:
On 4/21/22 13:14, Adam Ford wrote:
On Thu, Apr 21, 2022 at 5:29 AM Tommaso Merciai tommaso.merciai@amarulasolutions.com wrote:
On Thu, Apr 21, 2022 at 08:48:05AM +0200, Tommaso Merciai wrote:
- Fabio
- Tim
- Michael
- Marek
- Adam
Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the clk_enable to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this? I'm missing somethings?
I think the LCD driver should request the clock and clock rate based on settings the device tree. However, I think the bigger issues is that you might run into issues with the lack of a disp-blkctrl driver. Marek enable the GPC driver fairly recently, but the blkctrl driver will be needed to enable the LCD and DSI portions or the system may hang.
Just boot quickly and init the graphics pipeline in Linux ?
Hi Marek, Thanks for your feedback. You think make no sense to enable support for the graphics pipeline at U-Boot level? Some customers want this feature for that I think is better to have support for that. What do you think about?
I think it makes no sense to duplicate graphics pipeline in U-Boot.
Just boot quickly enough and bring the graphics pipeline in Linux, then maintain one copy of all the drivers involved in the pipeline (scanout engine, GPCs, block controllers, bridge drivers, panel drivers). Also, you won't have to deal with the display pipeline handoff from U-Boot to Linux, which incurs flicker.

On 21.04.22 14:01, Marek Vasut wrote:
On 4/21/22 13:54, Tommaso Merciai wrote:
On Thu, Apr 21, 2022 at 01:20:58PM +0200, Marek Vasut wrote:
On 4/21/22 13:14, Adam Ford wrote:
On Thu, Apr 21, 2022 at 5:29 AM Tommaso Merciai tommaso.merciai@amarulasolutions.com wrote:
On Thu, Apr 21, 2022 at 08:48:05AM +0200, Tommaso Merciai wrote:
- Fabio
- Tim
- Michael
- Marek
- Adam
Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the clk_enable to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this? I'm missing somethings?
I think the LCD driver should request the clock and clock rate based on settings the device tree. However, I think the bigger issues is that you might run into issues with the lack of a disp-blkctrl driver. Marek enable the GPC driver fairly recently, but the blkctrl driver will be needed to enable the LCD and DSI portions or the system may hang.
Just boot quickly and init the graphics pipeline in Linux ?
Hi Marek, Thanks for your feedback. You think make no sense to enable support for the graphics pipeline at U-Boot level? Some customers want this feature for that I think is better to have support for that. What do you think about?
I think it makes no sense to duplicate graphics pipeline in U-Boot.
Just boot quickly enough and bring the graphics pipeline in Linux,
+1
Rather, it is not time anymore to get a flickering-free display from U-Boot to Linux (it was possible on some SOC before DRM). Just booting fast is the best option.
Regards, Stefano
then maintain one copy of all the drivers involved in the pipeline (scanout engine, GPCs, block controllers, bridge drivers, panel drivers). Also, you won't have to deal with the display pipeline handoff from U-Boot to Linux, which incurs flicker.

On Thu, Apr 21, 2022 at 7:03 AM Stefano Babic sbabic@denx.de wrote:
On 21.04.22 14:01, Marek Vasut wrote:
On 4/21/22 13:54, Tommaso Merciai wrote:
On Thu, Apr 21, 2022 at 01:20:58PM +0200, Marek Vasut wrote:
On 4/21/22 13:14, Adam Ford wrote:
On Thu, Apr 21, 2022 at 5:29 AM Tommaso Merciai tommaso.merciai@amarulasolutions.com wrote:
On Thu, Apr 21, 2022 at 08:48:05AM +0200, Tommaso Merciai wrote:
- Fabio
- Tim
- Michael
- Marek
- Adam
> Hi, > I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up > eLCDIF > clocks. After port all necessary clocks needed by eLCDIF I found that > IMX8MM_VIDEO_PLL1 clock is not enabled and need the clk_enable to > enable > it at the end of the clk-imx8mm probe: > > struct clk *clkp; > > clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); > clk_set_rate(clkp, 594000000UL); > clk_enable(clkp); > > What do you think about this solution? > There is a more standard way to do this? > I'm missing somethings?
I think the LCD driver should request the clock and clock rate based on settings the device tree. However, I think the bigger issues is that you might run into issues with the lack of a disp-blkctrl driver. Marek enable the GPC driver fairly recently, but the blkctrl driver will be needed to enable the LCD and DSI portions or the system may hang.
Just boot quickly and init the graphics pipeline in Linux ?
Hi Marek, Thanks for your feedback. You think make no sense to enable support for the graphics pipeline at U-Boot level? Some customers want this feature for that I think is better to have support for that. What do you think about?
I think it makes no sense to duplicate graphics pipeline in U-Boot.
Just boot quickly enough and bring the graphics pipeline in Linux,
+1
Rather, it is not time anymore to get a flickering-free display from U-Boot to Linux (it was possible on some SOC before DRM). Just booting fast is the best option.
Regards, Stefano
then maintain one copy of all the drivers involved in the pipeline (scanout engine, GPCs, block controllers, bridge drivers, panel drivers). Also, you won't have to deal with the display pipeline handoff from U-Boot to Linux, which incurs flicker.
For what it's worth, there was a thread about using Falcon mode with ARM64. I was going to investigate it when I have some time. It would be nice to bypass the U-Boot phase completely and jump from SPL->ATF->Linux if possible.
adam
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de =====================================================================

Hi
On Thu, Apr 21, 2022 at 2:05 PM Adam Ford aford173@gmail.com wrote:
On Thu, Apr 21, 2022 at 7:03 AM Stefano Babic sbabic@denx.de wrote:
On 21.04.22 14:01, Marek Vasut wrote:
On 4/21/22 13:54, Tommaso Merciai wrote:
On Thu, Apr 21, 2022 at 01:20:58PM +0200, Marek Vasut wrote:
On 4/21/22 13:14, Adam Ford wrote:
On Thu, Apr 21, 2022 at 5:29 AM Tommaso Merciai tommaso.merciai@amarulasolutions.com wrote: > > On Thu, Apr 21, 2022 at 08:48:05AM +0200, Tommaso Merciai wrote: > > + Fabio > + Tim > + Michael > + Marek > + Adam > >> Hi, >> I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up >> eLCDIF >> clocks. After port all necessary clocks needed by eLCDIF I found that >> IMX8MM_VIDEO_PLL1 clock is not enabled and need the clk_enable to >> enable >> it at the end of the clk-imx8mm probe: >> >> struct clk *clkp; >> >> clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); >> clk_set_rate(clkp, 594000000UL); >> clk_enable(clkp); >> >> What do you think about this solution? >> There is a more standard way to do this? >> I'm missing somethings?
I think the LCD driver should request the clock and clock rate based on settings the device tree. However, I think the bigger issues is that you might run into issues with the lack of a disp-blkctrl driver. Marek enable the GPC driver fairly recently, but the blkctrl driver will be needed to enable the LCD and DSI portions or the system may hang.
Just boot quickly and init the graphics pipeline in Linux ?
Hi Marek, Thanks for your feedback. You think make no sense to enable support for the graphics pipeline at U-Boot level? Some customers want this feature for that I think is better to have support for that. What do you think about?
I think it makes no sense to duplicate graphics pipeline in U-Boot.
Just boot quickly enough and bring the graphics pipeline in Linux,
+1
Rather, it is not time anymore to get a flickering-free display from U-Boot to Linux (it was possible on some SOC before DRM). Just booting fast is the best option.
Regards, Stefano
then maintain one copy of all the drivers involved in the pipeline (scanout engine, GPCs, block controllers, bridge drivers, panel drivers). Also, you won't have to deal with the display pipeline handoff from U-Boot to Linux, which incurs flicker.
For what it's worth, there was a thread about using Falcon mode with ARM64. I was going to investigate it when I have some time. It would be nice to bypass the U-Boot phase completely and jump from SPL->ATF->Linux if possible.
adam
Well we should consider graphics panel in uboot, it's standard chain for mobile phone, or rockchip bsp
Michael
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de =====================================================================
-- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 michael@amarulasolutions.com __________________________________
Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 info@amarulasolutions.com www.amarulasolutions.com

On Thu, Apr 21, 2022 at 06:14:58AM -0500, Adam Ford wrote:
On Thu, Apr 21, 2022 at 5:29 AM Tommaso Merciai tommaso.merciai@amarulasolutions.com wrote:
On Thu, Apr 21, 2022 at 08:48:05AM +0200, Tommaso Merciai wrote:
- Fabio
- Tim
- Michael
- Marek
- Adam
Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the clk_enable to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this? I'm missing somethings?
I think the LCD driver should request the clock and clock rate based on settings the device tree. However, I think the bigger issues is that you might run into issues with the lack of a disp-blkctrl driver. Marek enable the GPC driver fairly recently, but the blkctrl driver will be needed to enable the LCD and DSI portions or the system may hang.
Hi Adam, Thanks for the tips, I will investigate on that.
Tommaso
adam
Thanks, Tommmaso -- Tommaso Merciai Embedded Linux Engineer tommaso.merciai@amarulasolutions.com __________________________________
Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@amarulasolutions.com www.amarulasolutions.com
-- Tommaso Merciai Embedded Linux Engineer tommaso.merciai@amarulasolutions.com __________________________________
Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@amarulasolutions.com www.amarulasolutions.com

On 4/21/22 2:48 AM, Tommaso Merciai wrote:
Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the following code to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this?
PLL1 should be a parent of one of the clocks required by the eLCDIF. That clock should call clk_enable() on PLL1 when it is enabled itself. If you want to set a specific rate, you can do that with assigned-clock-rates in either the clock's DT node, or the eLCDIF's DT node.
--Sean

On Thu, Apr 21, 2022 at 08:09:59PM -0400, Sean Anderson wrote:
On 4/21/22 2:48 AM, Tommaso Merciai wrote:
Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the following code to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this?
PLL1 should be a parent of one of the clocks required by the eLCDIF. That clock should call clk_enable() on PLL1 when it is enabled itself. If you want to set a specific rate, you can do that with assigned-clock-rates in either the clock's DT node, or the eLCDIF's DT node.
Hi Sean, Thanks for your suggestion, I need only to enable it, I have already assign the right rate from dts. The doubt at this point is: it's right call clk_enable from clk-imx8mm.c? I think yes because it handle by "fsl,imx8mm-ccm" driver, maybe protected by CONFIG_DM_VIDEO could be a good solution. Let me know.
Thanks, Tommaso
--Sean

On 4/22/22 2:39 AM, Tommaso Merciai wrote:
On Thu, Apr 21, 2022 at 08:09:59PM -0400, Sean Anderson wrote:
On 4/21/22 2:48 AM, Tommaso Merciai wrote:
Hi, I'm working on drivers/clk/imx/clk-imx8mm.c to port and bring up eLCDIF clocks. After port all necessary clocks needed by eLCDIF I found that IMX8MM_VIDEO_PLL1 clock is not enabled and need the following code to enable it at the end of the clk-imx8mm probe:
struct clk *clkp;
clk_get_by_id(IMX8MM_VIDEO_PLL1, &clkp); clk_set_rate(clkp, 594000000UL); clk_enable(clkp);
What do you think about this solution? There is a more standard way to do this?
PLL1 should be a parent of one of the clocks required by the eLCDIF. That clock should call clk_enable() on PLL1 when it is enabled itself. If you want to set a specific rate, you can do that with assigned-clock-rates in either the clock's DT node, or the eLCDIF's DT node.
Hi Sean, Thanks for your suggestion, I need only to enable it, I have already assign the right rate from dts. The doubt at this point is: it's right call clk_enable from clk-imx8mm.c? I think yes because it handle by "fsl,imx8mm-ccm" driver, maybe protected by CONFIG_DM_VIDEO could be a good solution. Let me know.
I don't think so. Generally whatever uses the clock (e.g. your video driver) will call clk_enable, and CCF should propegate that enable up the clock tree.
--Sean
participants (6)
-
Adam Ford
-
Marek Vasut
-
Michael Nazzareno Trimarchi
-
Sean Anderson
-
Stefano Babic
-
Tommaso Merciai