diff mbox series

[1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel

Message ID 20200414200017.226136-2-martin.blumenstingl@googlemail.com (mailing list archive)
State New, archived
Headers show
Series clk: meson8b: updates for video clocks / resets | expand

Commit Message

Martin Blumenstingl April 14, 2020, 8 p.m. UTC
Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
easy to see that the vendor kernel does the same, but it actually does.
meson_clk_pll_ops in mainline still cannot fully recalculate all rates
from the HDMI PLL registers because some register bits (at the time of
writing it's unknown which bits are used for this) double the HDMI PLL
output rate (compared to simply considering M, N and FRAC).

Update the vid_pll_in_sel parent so our clock calculation works for
simple clock settings like the CVBS output (where no rate doubling is
going on). The PLL ops need to be fixed later on for more complex clock
settings (all HDMI rates).

Fixes: 6cb57c678bb70 ("clk: meson: meson8b: add the read-only video clock trees")
Suggested-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jerome Brunet April 16, 2020, 10:32 a.m. UTC | #1
On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
> easy to see that the vendor kernel does the same, but it actually does.
> meson_clk_pll_ops in mainline still cannot fully recalculate all rates
> from the HDMI PLL registers because some register bits (at the time of
> writing it's unknown which bits are used for this) double the HDMI PLL
> output rate (compared to simply considering M, N and FRAC).

Have you considered adding a fixed_factor pre-multiplier, like in the
gxbb driver ?

Seems to be the same thing

>
> Update the vid_pll_in_sel parent so our clock calculation works for
> simple clock settings like the CVBS output (where no rate doubling is
> going on). The PLL ops need to be fixed later on for more complex clock
> settings (all HDMI rates).
>
> Fixes: 6cb57c678bb70 ("clk: meson: meson8b: add the read-only video clock trees")
> Suggested-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/clk/meson/meson8b.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> index 7c55c695cbae..90d284ffc780 100644
> --- a/drivers/clk/meson/meson8b.c
> +++ b/drivers/clk/meson/meson8b.c
> @@ -1077,7 +1077,7 @@ static struct clk_regmap meson8b_vid_pll_in_sel = {
>  		 * Meson8m2: vid2_pll
>  		 */
>  		.parent_hws = (const struct clk_hw *[]) {
> -			&meson8b_hdmi_pll_dco.hw
> +			&meson8b_hdmi_pll_lvds_out.hw
>  		},
>  		.num_parents = 1,
>  		.flags = CLK_SET_RATE_PARENT,
Martin Blumenstingl April 16, 2020, 5:49 p.m. UTC | #2
Hi Jerome,

On Thu, Apr 16, 2020 at 12:32 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>
> > Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
> > easy to see that the vendor kernel does the same, but it actually does.
> > meson_clk_pll_ops in mainline still cannot fully recalculate all rates
> > from the HDMI PLL registers because some register bits (at the time of
> > writing it's unknown which bits are used for this) double the HDMI PLL
> > output rate (compared to simply considering M, N and FRAC).
>
> Have you considered adding a fixed_factor pre-multiplier, like in the
> gxbb driver ?
>
> Seems to be the same thing
it seems like I haven't been clear enough here: the doubling only
happens for some - but not all - PLL settings.

Let me give you two examples with values from the 3.10 vendor kernel:
1. the CVBS modes use a hdmi_pll_dco freq of 1296MHz
it uses: M = 54, N = 1 and FRAC = 0
with our existing clock tree this works out perfectly: 24MHz * 54 / 1 = 1296MHz

2. HDMI 1080P mode uses hdmi_pll_dco freq of 2970MHz
it uses M = 61, N = 1 and FRAC = 3584
with our existing clock tree this doesn't end up right: (24MHz * 61 /
1)  + (24MHz * 3584 / 4095) = 1485MHz

I did play with the registers and our clock-measurer.
it *seems* that the HHI_VID_PLL_CNTL3 and/or HHI_VID_PLL_CNTL4 are
related to this doubling, but I don't know for sure
my assumption is that there's either a fixed pre-multiplier like you
suggested and then a configurable "divide by 2" somewhere or there's
simply a configurable "multiply by 2" somewhere
Either way, I want to fix that at some point but since I don't know
the related bits I want to do that later on (in separate patches once
I have figured it out)


Regards
Martin
diff mbox series

Patch

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 7c55c695cbae..90d284ffc780 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -1077,7 +1077,7 @@  static struct clk_regmap meson8b_vid_pll_in_sel = {
 		 * Meson8m2: vid2_pll
 		 */
 		.parent_hws = (const struct clk_hw *[]) {
-			&meson8b_hdmi_pll_dco.hw
+			&meson8b_hdmi_pll_lvds_out.hw
 		},
 		.num_parents = 1,
 		.flags = CLK_SET_RATE_PARENT,