imx-drm: Add HDMI support
Shawn Guo
shawn.guo at linaro.org
Mon Oct 21 05:13:03 EDT 2013
On Sat, Oct 19, 2013 at 01:12:18PM +0100, Russell King - ARM Linux wrote:
> Okay, more on this.
>
> If I switch to using the DI clock, sourced from PLL5, and then "fix"
> the clock tree such that I can change PLL5's rate, then I get most of
> the display modes working.
>
> However, it will occasionally not work, and when that happens I need
> to reboot to get any kind of output working again. I haven't delved
> into it with the scope to find out what the HDMI clock is doing yet.
>
> Having solved it, what I think is going on is as follows:
>
> When one of the PLLs is prepared and then subsequently enabled, the
> following sequence is used:
>
> - Clear bypass
> - Power up the PLL
> - Wait for the PLL to indicate lock
> - Enable output
>
> This causes problems for me. If I change this to:
>
> - Power up the PLL
> - Wait for PLL to indicate lock
> - Clear bypass
> - Enable output
>
> then I don't see any problems. My theory is that the bypass mux and/or
> enable gate settings are synchronised with the mux output, and if the
> mux output glitches while the PLL is gaining lock, it knocks that out
> preventing the output from ever being enabled until the chip is hit with
> a reset.
You're right. I just checked FSL 3.0.35 kernel and found that BYPASS
bit is written after POWER bit.
> The other issue is that the set_rate() functions don't wait for the PLL
> to lock before returning, and don't set bypass mode. It looks like this
> code relies on drivers unpreparing the clocks before calling clk_set_rate().
> I don't see the clk API enforcing this in any way, so I'd suggest that
> the set_rate() function also does the bypass -> set rate -> wait lock
> -> clear bypass transition too. Maybe this should be applied to the
> other PLLs too.
Yes. We experienced the problem when upgrading FSL kernel tree to 3.10.
The .set_rate() should wait for LOCK bit, but does not need to touch
BYPASS.
> Here's a diff which updates the av PLL to do this:
Some other PLLs needs update too. I will cook up patches to fix these
problems.
Shawn
> arch/arm/mach-imx/clk-pllv3.c | 77 ++++++++++++++++++++++++++++------------
> 1 files changed, 54 insertions(+), 23 deletions(-)
>
> diff --git a/arch/arm/mach-imx/clk-pllv3.c b/arch/arm/mach-imx/clk-pllv3.c
> index f6640b6..11a7048 100644
> --- a/arch/arm/mach-imx/clk-pllv3.c
> +++ b/arch/arm/mach-imx/clk-pllv3.c
> @@ -45,21 +45,10 @@ struct clk_pllv3 {
>
> #define to_clk_pllv3(_hw) container_of(_hw, struct clk_pllv3, hw)
>
> -static int clk_pllv3_prepare(struct clk_hw *hw)
> +static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
> {
> - struct clk_pllv3 *pll = to_clk_pllv3(hw);
> - unsigned long timeout;
> - u32 val;
> -
> - val = readl_relaxed(pll->base);
> - val &= ~BM_PLL_BYPASS;
> - if (pll->powerup_set)
> - val |= BM_PLL_POWER;
> - else
> - val &= ~BM_PLL_POWER;
> - writel_relaxed(val, pll->base);
> + unsigned long timeout = jiffies + msecs_to_jiffies(100);
>
> - timeout = jiffies + msecs_to_jiffies(10);
> /* Wait for PLL to lock */
> do {
> if (readl_relaxed(pll->base) & BM_PLL_LOCK)
> @@ -68,10 +57,31 @@ static int clk_pllv3_prepare(struct clk_hw *hw)
> break;
> } while (1);
>
> - if (readl_relaxed(pll->base) & BM_PLL_LOCK)
> - return 0;
> + return readl_relaxed(pll->base) & BM_PLL_LOCK ? 0 : -ETIMEDOUT;
> +}
> +
> +static int clk_pllv3_prepare(struct clk_hw *hw)
> +{
> + struct clk_pllv3 *pll = to_clk_pllv3(hw);
> + u32 val, newval;
> + int ret;
> +
> + val = readl_relaxed(pll->base);
> + if (pll->powerup_set)
> + newval = val | BM_PLL_POWER;
> else
> - return -ETIMEDOUT;
> + newval = val & ~BM_PLL_POWER;
> + writel_relaxed(newval, pll->base);
> +
> + ret = clk_pllv3_wait_lock(pll);
> + if (ret == 0) {
> + newval &= ~BM_PLL_BYPASS;
> + writel_relaxed(newval, pll->base);
> + } else {
> + writel_relaxed(val, pll->base);
> + }
> +
> + return ret;
> }
>
> static void clk_pllv3_unprepare(struct clk_hw *hw)
> @@ -80,6 +90,7 @@ static void clk_pllv3_unprepare(struct clk_hw *hw)
> u32 val;
>
> val = readl_relaxed(pll->base);
> + pr_info("%s: 0x%08x\n", __func__, val);
> val |= BM_PLL_BYPASS;
> if (pll->powerup_set)
> val &= ~BM_PLL_POWER;
> @@ -256,9 +267,10 @@ static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,
> struct clk_pllv3 *pll = to_clk_pllv3(hw);
> unsigned long min_rate = parent_rate * 27;
> unsigned long max_rate = parent_rate * 54;
> - u32 val, div;
> + u32 val, newval, div;
> u32 mfn, mfd = 1000000;
> s64 temp64;
> + int ret;
>
> if (rate < min_rate || rate > max_rate)
> return -EINVAL;
> @@ -270,13 +282,32 @@ static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,
> mfn = temp64;
>
> val = readl_relaxed(pll->base);
> - val &= ~pll->div_mask;
> - val |= div;
> - writel_relaxed(val, pll->base);
> +
> + /* set the PLL into bypass mode */
> + newval = val | BM_PLL_BYPASS;
> + writel_relaxed(newval, pll->base);
> +
> + /* configure the new frequency */
> + newval &= ~pll->div_mask;
> + newval |= div;
> + writel_relaxed(newval, pll->base);
> writel_relaxed(mfn, pll->base + PLL_NUM_OFFSET);
> - writel_relaxed(mfd, pll->base + PLL_DENOM_OFFSET);
> + writel(mfd, pll->base + PLL_DENOM_OFFSET);
> +
> + if (val & BM_PLL_POWER) {
> + /* PLL is powered down */
> + ret = 0;
> + } else {
> + ret = clk_pllv3_wait_lock(pll);
> + if (ret == 0) {
> + /* only if it locked can we switch back to the PLL */
> + newval &= ~BM_PLL_BYPASS;
> + newval |= val & BM_PLL_BYPASS;
> + writel(newval, pll->base);
> + }
> + }
>
> - return 0;
> + return ret;
> }
>
> static const struct clk_ops clk_pllv3_av_ops = {
>
More information about the linux-arm-kernel
mailing list