MXSFB and Video PLL clock on i.MX8M Mini/Nano Question

MXSFB and Video PLL clock on i.MX8M Mini/Nano Question

[Adam Ford]

I am trying to work through a series that was submitted for enabling
the DSI on the i.MX8M Mini and Nano.  I have extended this series to
route the DSI to an HDMI bridge, and I am able to get several
resolutions to properly sync on my monitor.  However, there are also a
 bunch that appear on the list when I run modetest that do not sync on
my monitor.

When running some debug code, it appears that it's related to the
clocking of the MXSFB driver.

From what I can tell, the MSXFB driver attempts to set the clock based
on the desired resolution and refresh rate.  When the default
VIDEO_PLL clock is set to 594MHz, many of the resolutions that cleanly
divide from the 594MHz clock appear to sync with my monitor.  However,
in order to get other resolutions to appear, I have to manually change
the device tree to set VIDEO_PLL to a different clock rate so MSXFB
can use it.  Unfortunately, that breaks the resolutions that used to
work.

I threw together a hack into the MXSFB driver which adds a new
optional clock to the MSXFB driver.  When I pass VIDEO_PLL to this
driver, it can automatically set the clock rate to match that of
whatever the desired clock is, and I can get many more resolutions to
appear.
Another advantage of this is that the Video_PLL can be the minimum
speed needed for a given rate instead of setting a higher rate, then
dividing it down.

I wasn't sure if that would be the best way to approach this, or if
someone had a better suggestion.

If this is the best approach, I can try to clean up my patch and
submit it to the mailing list for review.

thanks,

adam

Re: MXSFB and Video PLL clock on i.MX8M Mini/Nano Question

[Sascha Hauer]

On Sun, Mar 12, 2023 at 02:28:45PM -0500, Adam Ford wrote:
> I am trying to work through a series that was submitted for enabling
> the DSI on the i.MX8M Mini and Nano.  I have extended this series to
> route the DSI to an HDMI bridge, and I am able to get several
> resolutions to properly sync on my monitor.  However, there are also a
>  bunch that appear on the list when I run modetest that do not sync on
> my monitor.
> 
> When running some debug code, it appears that it's related to the
> clocking of the MXSFB driver.
> 
> From what I can tell, the MSXFB driver attempts to set the clock based
> on the desired resolution and refresh rate.  When the default
> VIDEO_PLL clock is set to 594MHz, many of the resolutions that cleanly
> divide from the 594MHz clock appear to sync with my monitor.  However,
> in order to get other resolutions to appear, I have to manually change
> the device tree to set VIDEO_PLL to a different clock rate so MSXFB
> can use it.  Unfortunately, that breaks the resolutions that used to
> work.
> 
> I threw together a hack into the MXSFB driver which adds a new
> optional clock to the MSXFB driver.  When I pass VIDEO_PLL to this
> driver, it can automatically set the clock rate to match that of
> whatever the desired clock is, and I can get many more resolutions to
> appear.
> Another advantage of this is that the Video_PLL can be the minimum
> speed needed for a given rate instead of setting a higher rate, then
> dividing it down.

Isn't it possible to add the CLK_SET_RATE_PARENT flag to the pixel
clock? That's what i.MX6sx and i.MX7 do.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: MXSFB and Video PLL clock on i.MX8M Mini/Nano Question

[Adam Ford]

On Mon, Mar 13, 2023 at 3:51 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Sun, Mar 12, 2023 at 02:28:45PM -0500, Adam Ford wrote:
> > I am trying to work through a series that was submitted for enabling
> > the DSI on the i.MX8M Mini and Nano.  I have extended this series to
> > route the DSI to an HDMI bridge, and I am able to get several
> > resolutions to properly sync on my monitor.  However, there are also a
> >  bunch that appear on the list when I run modetest that do not sync on
> > my monitor.
> >
> > When running some debug code, it appears that it's related to the
> > clocking of the MXSFB driver.
> >
> > From what I can tell, the MSXFB driver attempts to set the clock based
> > on the desired resolution and refresh rate.  When the default
> > VIDEO_PLL clock is set to 594MHz, many of the resolutions that cleanly
> > divide from the 594MHz clock appear to sync with my monitor.  However,
> > in order to get other resolutions to appear, I have to manually change
> > the device tree to set VIDEO_PLL to a different clock rate so MSXFB
> > can use it.  Unfortunately, that breaks the resolutions that used to
> > work.
> >
> > I threw together a hack into the MXSFB driver which adds a new
> > optional clock to the MSXFB driver.  When I pass VIDEO_PLL to this
> > driver, it can automatically set the clock rate to match that of
> > whatever the desired clock is, and I can get many more resolutions to
> > appear.
> > Another advantage of this is that the Video_PLL can be the minimum
> > speed needed for a given rate instead of setting a higher rate, then
> > dividing it down.
>
> Isn't it possible to add the CLK_SET_RATE_PARENT flag to the pixel
> clock? That's what i.MX6sx and i.MX7 do.

I thought about that, but on the Nano, the video_pll is several layers up.

video_pll -> video_pll_bypass -> disp_pixel -> disp_pixel_clk

Do I just set that flag for each of these?

adam
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: MXSFB and Video PLL clock on i.MX8M Mini/Nano Question

[Sascha Hauer]

On Mon, Mar 13, 2023 at 06:08:05AM -0500, Adam Ford wrote:
> On Mon, Mar 13, 2023 at 3:51 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > On Sun, Mar 12, 2023 at 02:28:45PM -0500, Adam Ford wrote:
> > > I am trying to work through a series that was submitted for enabling
> > > the DSI on the i.MX8M Mini and Nano.  I have extended this series to
> > > route the DSI to an HDMI bridge, and I am able to get several
> > > resolutions to properly sync on my monitor.  However, there are also a
> > >  bunch that appear on the list when I run modetest that do not sync on
> > > my monitor.
> > >
> > > When running some debug code, it appears that it's related to the
> > > clocking of the MXSFB driver.
> > >
> > > From what I can tell, the MSXFB driver attempts to set the clock based
> > > on the desired resolution and refresh rate.  When the default
> > > VIDEO_PLL clock is set to 594MHz, many of the resolutions that cleanly
> > > divide from the 594MHz clock appear to sync with my monitor.  However,
> > > in order to get other resolutions to appear, I have to manually change
> > > the device tree to set VIDEO_PLL to a different clock rate so MSXFB
> > > can use it.  Unfortunately, that breaks the resolutions that used to
> > > work.
> > >
> > > I threw together a hack into the MXSFB driver which adds a new
> > > optional clock to the MSXFB driver.  When I pass VIDEO_PLL to this
> > > driver, it can automatically set the clock rate to match that of
> > > whatever the desired clock is, and I can get many more resolutions to
> > > appear.
> > > Another advantage of this is that the Video_PLL can be the minimum
> > > speed needed for a given rate instead of setting a higher rate, then
> > > dividing it down.
> >
> > Isn't it possible to add the CLK_SET_RATE_PARENT flag to the pixel
> > clock? That's what i.MX6sx and i.MX7 do.
> 
> I thought about that, but on the Nano, the video_pll is several layers up.
> 
> video_pll -> video_pll_bypass -> disp_pixel -> disp_pixel_clk
> 
> Do I just set that flag for each of these?

Yes, that's what I would suggest. I don't know the i.MX8M clock tree
very well, so I don't know which other clocks might be influenced when
the video_pll clock changes its rate. But if you have to change the PLL
rate anyway it shouldn't make a difference if you change it directly
or if you let the rate change propagate up from disp_pixel_clk.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: MXSFB and Video PLL clock on i.MX8M Mini/Nano Question

[Lucas Stach]

Am Montag, dem 13.03.2023 um 12:29 +0100 schrieb Sascha Hauer:
> On Mon, Mar 13, 2023 at 06:08:05AM -0500, Adam Ford wrote:
> > On Mon, Mar 13, 2023 at 3:51 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > 
> > > On Sun, Mar 12, 2023 at 02:28:45PM -0500, Adam Ford wrote:
> > > > I am trying to work through a series that was submitted for enabling
> > > > the DSI on the i.MX8M Mini and Nano.  I have extended this series to
> > > > route the DSI to an HDMI bridge, and I am able to get several
> > > > resolutions to properly sync on my monitor.  However, there are also a
> > > >  bunch that appear on the list when I run modetest that do not sync on
> > > > my monitor.
> > > > 
> > > > When running some debug code, it appears that it's related to the
> > > > clocking of the MXSFB driver.
> > > > 
> > > > From what I can tell, the MSXFB driver attempts to set the clock based
> > > > on the desired resolution and refresh rate.  When the default
> > > > VIDEO_PLL clock is set to 594MHz, many of the resolutions that cleanly
> > > > divide from the 594MHz clock appear to sync with my monitor.  However,
> > > > in order to get other resolutions to appear, I have to manually change
> > > > the device tree to set VIDEO_PLL to a different clock rate so MSXFB
> > > > can use it.  Unfortunately, that breaks the resolutions that used to
> > > > work.
> > > > 
> > > > I threw together a hack into the MXSFB driver which adds a new
> > > > optional clock to the MSXFB driver.  When I pass VIDEO_PLL to this
> > > > driver, it can automatically set the clock rate to match that of
> > > > whatever the desired clock is, and I can get many more resolutions to
> > > > appear.
> > > > Another advantage of this is that the Video_PLL can be the minimum
> > > > speed needed for a given rate instead of setting a higher rate, then
> > > > dividing it down.
> > > 
> > > Isn't it possible to add the CLK_SET_RATE_PARENT flag to the pixel
> > > clock? That's what i.MX6sx and i.MX7 do.
> > 
> > I thought about that, but on the Nano, the video_pll is several layers up.
> > 
> > video_pll -> video_pll_bypass -> disp_pixel -> disp_pixel_clk
> > 
> > Do I just set that flag for each of these?
> 
> Yes, that's what I would suggest. I don't know the i.MX8M clock tree
> very well, so I don't know which other clocks might be influenced when
> the video_pll clock changes its rate. But if you have to change the PLL
> rate anyway it shouldn't make a difference if you change it directly
> or if you let the rate change propagate up from disp_pixel_clk.
> 
The problem with a simple CLK_SET_RATE_PARENT is that the rate changes
propagate upwards in the tree for all possible sources. If the source
is the video PLL then the rate propagation is exactly what you want to
happen. If the display clock is sourced from a system PLL, you really
don't want to change the PLL rate, so we need something more clever
here.

Either the mux part of the composite clock needs to learn to only
propagate rate requests for specific sources, or we need to deny rate
changes to the system PLLs, but not sure if there are other similar
cases for other sources.

Regards,
Lucas