imx8mm eLCDIF clock

imx8mm eLCDIF clock

[Tommaso Merciai]

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
-- 
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

Re: imx8mm eLCDIF clock

[Tommaso Merciai]

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

-- 
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

Re: imx8mm eLCDIF clock

[Adam Ford]

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

Re: imx8mm eLCDIF clock

[Marek Vasut]

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 ?

Re: imx8mm eLCDIF clock

[Tommaso Merciai]

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

-- 
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

Re: imx8mm eLCDIF clock

[Tommaso Merciai]

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

-- 
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

Re: imx8mm eLCDIF clock

[Marek Vasut]

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.

Re: imx8mm eLCDIF clock

[Stefano Babic]

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.




-- 
=====================================================================
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
=====================================================================

Re: imx8mm eLCDIF clock

[Adam Ford]

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
> =====================================================================

Re: imx8mm eLCDIF clock

[Michael Nazzareno Trimarchi]

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

Re: imx8mm eLCDIF clock

[Sean Anderson]

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

Re: imx8mm eLCDIF clock

[Tommaso Merciai]

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

-- 
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

Re: imx8mm eLCDIF clock

[Sean Anderson]

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