From patchwork Sat Oct 19 12:12:18 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 284868 Return-Path: X-Original-To: incoming-imx@patchwork.ozlabs.org Delivered-To: patchwork-incoming-imx@bilbo.ozlabs.org Received: from casper.infradead.org (unknown [IPv6:2001:770:15f::2]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id C8BFE2C0126 for ; Sat, 19 Oct 2013 23:13:54 +1100 (EST) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VXVPF-0002ry-Is; Sat, 19 Oct 2013 12:13:21 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VXVPD-0002hY-66; Sat, 19 Oct 2013 12:13:19 +0000 Received: from [2002:4e20:1eda::1] (helo=caramon.arm.linux.org.uk) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VXVP8-0002h1-Cd for linux-arm-kernel@lists.infradead.org; Sat, 19 Oct 2013 12:13:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=caramon; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=StYnnlSuZ/dmjBrYybXUJN4r47f5+68PgVXl+36GRc8=; b=Jt89tzqvCU6ibR1DO+H9aalEUNf6U+4QszcN7uKfqiI4Uj2i1BBzEq1slfrF/YRWcWBdtPANdNdQOCbJdvQ/DdUYY++QfrttfFjs3nbOIZxCzKB62knuQTXxd8MtGBDq8fstKp44unYYwbzi3kRuZp9lgQy98g+6RlyN4jtggpM=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:41588) by caramon.arm.linux.org.uk with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.76) (envelope-from ) id 1VXVOG-0000yM-JF; Sat, 19 Oct 2013 13:12:21 +0100 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1VXVOE-0004qa-Ph; Sat, 19 Oct 2013 13:12:18 +0100 Date: Sat, 19 Oct 2013 13:12:18 +0100 From: Russell King - ARM Linux To: Alexander Shiyan , Sascha Hauer , Shawn Guo Subject: Re: imx-drm: Add HDMI support Message-ID: <20131019121218.GH25034@n2100.arm.linux.org.uk> References: <52602EEA.6060402@prisktech.co.nz> <20131017193948.GT25034@n2100.arm.linux.org.uk> <1382039535.976669051@f130.i.mail.ru> <20131017203545.GV25034@n2100.arm.linux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20131017203545.GV25034@n2100.arm.linux.org.uk> User-Agent: Mutt/1.5.19 (2009-01-05) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131019_081315_164474_74FC2B12 X-CRM114-Status: GOOD ( 27.30 ) X-Spam-Score: -1.2 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.8 RDNS_NONE Delivered to internal network by a host with no rDNS Cc: Fabio Estevam , "Mailing List, Arm" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org List-Id: linux-imx-kernel.lists.patchwork.ozlabs.org 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. 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. Here's a diff which updates the av PLL to do this: 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 = {