[U-Boot] [PATCH] sunxi: video: HDMI: Fix clock setup

[Jagan Teki]

On Sun, Mar 24, 2019 at 11:56 PM Jernej Skrabec <jernej.skrabec at siol.net> wrote:
>
> Currently, HDMI driver doesn't consider minimum and maximum allowed rate
> of pll3 (video PLL). It works most of the time, but not always.
>
> Consider monitor with resolution 1920x1200, which has pixel clock rate
> of 154 MHz. Current code would determine that pll3 rate has to be set to
> 154 MHz. However, minimum supported rate is 192 MHz. In this case video
> output just won't work.

I set the monitor with same resolution, but not sure with the pixel
clock and unable to reproduce this with couple time. any specific
usage case that the issue will reproduce ?

>
> The reason why the driver is written in the way it is, is that at the
> time HDMI PHY and clock configuration wasn't fully understood. But now
> we have needed knowledge, so the issue can be fixed.
>
> With this fix, clock configuration routine uses full range (1-16) for
> clock divider instead of limited one (1, 2, 4, 11). It also considers
> minimum and maximum allowed rate for pll3.
>
> Fixes: 56009451d843 ("sunxi: video: Add A64/H3/H5 HDMI driver")
> Signed-off-by: Jernej Skrabec <jernej.skrabec at siol.net>
> ---
> Hi!
>
> Unfortunately, issue fixed with patch here has influence on Linux too.
> In Linux, minimum and maximum rate is considered only when changing rate.
> But because U-Boot already set PLL to "correct" value, Linux clock driver
> doesn't see the need to change it. I know that this is Linux driver issue
> but let's start at the source of the issue and fix it.
>
> It would be very nice if this can go in 2019.04 release.
>
> Best regards,
> Jernej
>
>  drivers/video/sunxi/sunxi_dw_hdmi.c | 62 +++++++++++++++++------------
>  1 file changed, 37 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/video/sunxi/sunxi_dw_hdmi.c b/drivers/video/sunxi/sunxi_dw_hdmi.c
> index 9dbea649a0..6fe1aa7ee4 100644
> --- a/drivers/video/sunxi/sunxi_dw_hdmi.c
> +++ b/drivers/video/sunxi/sunxi_dw_hdmi.c
> @@ -132,7 +132,7 @@ static int sunxi_dw_hdmi_wait_for_hpd(void)
>         return -1;
>  }
>
> -static void sunxi_dw_hdmi_phy_set(uint clock)
> +static void sunxi_dw_hdmi_phy_set(uint clock, int phy_div)
>  {
>         struct sunxi_hdmi_phy * const phy =
>                 (struct sunxi_hdmi_phy *)(SUNXI_HDMI_BASE + HDMI_PHY_OFFS);
> @@ -146,7 +146,7 @@ static void sunxi_dw_hdmi_phy_set(uint clock)
>         switch (div) {
>         case 1:
>                 writel(0x30dc5fc0, &phy->pll);
> -               writel(0x800863C0, &phy->clk);
> +               writel(0x800863C0 | (phy_div - 1), &phy->clk);
>                 mdelay(10);
>                 writel(0x00000001, &phy->unk3);
>                 setbits_le32(&phy->pll, BIT(25));
> @@ -164,7 +164,7 @@ static void sunxi_dw_hdmi_phy_set(uint clock)
>                 break;
>         case 2:
>                 writel(0x39dc5040, &phy->pll);
> -               writel(0x80084381, &phy->clk);
> +               writel(0x80084380 | (phy_div - 1), &phy->clk);
>                 mdelay(10);
>                 writel(0x00000001, &phy->unk3);
>                 setbits_le32(&phy->pll, BIT(25));
> @@ -178,7 +178,7 @@ static void sunxi_dw_hdmi_phy_set(uint clock)
>                 break;
>         case 4:
>                 writel(0x39dc5040, &phy->pll);
> -               writel(0x80084343, &phy->clk);
> +               writel(0x80084340 | (phy_div - 1), &phy->clk);
>                 mdelay(10);
>                 writel(0x00000001, &phy->unk3);
>                 setbits_le32(&phy->pll, BIT(25));
> @@ -192,7 +192,7 @@ static void sunxi_dw_hdmi_phy_set(uint clock)
>                 break;
>         case 11:
>                 writel(0x39dc5040, &phy->pll);
> -               writel(0x8008430a, &phy->clk);
> +               writel(0x80084300 | (phy_div - 1), &phy->clk);
>                 mdelay(10);
>                 writel(0x00000001, &phy->unk3);
>                 setbits_le32(&phy->pll, BIT(25));
> @@ -207,36 +207,46 @@ static void sunxi_dw_hdmi_phy_set(uint clock)
>         }
>  }
>
> -static void sunxi_dw_hdmi_pll_set(uint clk_khz)
> +static void sunxi_dw_hdmi_pll_set(uint clk_khz, int *phy_div)
>  {
> -       int value, n, m, div = 0, diff;
> -       int best_n = 0, best_m = 0, best_diff = 0x0FFFFFFF;
> -
> -       div = sunxi_dw_hdmi_get_divider(clk_khz * 1000);
> +       int value, n, m, div, diff;
> +       int best_n = 0, best_m = 0, best_div = 0, best_diff = 0x0FFFFFFF;
>
>         /*
>          * Find the lowest divider resulting in a matching clock. If there
>          * is no match, pick the closest lower clock, as monitors tend to
>          * not sync to higher frequencies.
>          */
> -       for (m = 1; m <= 16; m++) {
> -               n = (m * div * clk_khz) / 24000;
> -
> -               if ((n >= 1) && (n <= 128)) {
> -                       value = (24000 * n) / m / div;
> -                       diff = clk_khz - value;
> -                       if (diff < best_diff) {
> -                               best_diff = diff;
> -                               best_m = m;
> -                               best_n = n;
> +       for (div = 1; div <= 16; div++) {
> +               int target = clk_khz * div;
> +
> +               if (target < 192000)
> +                       continue;
> +               if (target > 912000)
> +                       continue;

I think this can be max allowed rate by PLL_VIDEO0 can't it be 1008 or 600 ?