Hi Laurent,
On 01-03-2017 22:39, Laurent Pinchart wrote:
> When powering the PHY up we need to wait for the PLL to lock. This is
> done by polling the TX_PHY_LOCK bit in the HDMI_PHY_STAT0 register
> (interrupt-based wait could be implemented as well but is likely
> overkill). The bit is asserted when the PLL locks, but the current code
> incorrectly waits for the bit to be deasserted. Fix it, and while at it,
> replace the udelay() with a sleep as the code never runs in
> non-sleepable context.
>
> To be consistent with the power down implementation move the poll loop
> to the power off function.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
Reviewed-by: Jose Abreu <joabreu@xxxxxxxxxxxx>
Best regards,
Jose Miguel Abreu
> ---
> drivers/gpu/drm/bridge/dw-hdmi.c | 65 +++++++++++++++++++++++-----------------
> 1 file changed, 37 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 85348ba6bb1c..0aa3ad404f77 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -949,9 +949,44 @@ static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> dw_hdmi_phy_gen2_pddq(hdmi, 1);
> }
>
> +static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
> +{
> + const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> + unsigned int i;
> + u8 val;
> +
> + if (phy->gen == 1) {
> + dw_hdmi_phy_enable_powerdown(hdmi, false);
> +
> + /* Toggle TMDS enable. */
> + dw_hdmi_phy_enable_tmds(hdmi, 0);
> + dw_hdmi_phy_enable_tmds(hdmi, 1);
> + return 0;
> + }
> +
> + dw_hdmi_phy_gen2_txpwron(hdmi, 1);
> + dw_hdmi_phy_gen2_pddq(hdmi, 0);
> +
> + /* Wait for PHY PLL lock */
> + for (i = 0; i < 5; ++i) {
> + val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
> + if (val)
> + break;
> +
> + usleep_range(1000, 2000);
> + }
> +
> + if (!val) {
> + dev_err(hdmi->dev, "PHY PLL failed to lock\n");
> + return -ETIMEDOUT;
> + }
> +
> + dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
> + return 0;
> +}
> +
> static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> {
> - u8 val, msec;
> const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
> const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
> const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
> @@ -1019,33 +1054,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> hdmi_phy_i2c_write(hdmi, HDMI_3D_TX_PHY_CKCALCTRL_OVERRIDE,
> HDMI_3D_TX_PHY_CKCALCTRL);
>
> - dw_hdmi_phy_enable_powerdown(hdmi, false);
> -
> - /* toggle TMDS enable */
> - dw_hdmi_phy_enable_tmds(hdmi, 0);
> - dw_hdmi_phy_enable_tmds(hdmi, 1);
> -
> - /* gen2 tx power on */
> - dw_hdmi_phy_gen2_txpwron(hdmi, 1);
> - dw_hdmi_phy_gen2_pddq(hdmi, 0);
> -
> - /* Wait for PHY PLL lock */
> - msec = 5;
> - do {
> - val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
> - if (!val)
> - break;
> -
> - if (msec == 0) {
> - dev_err(hdmi->dev, "PHY PLL not locked\n");
> - return -ETIMEDOUT;
> - }
> -
> - udelay(1000);
> - msec--;
> - } while (1);
> -
> - return 0;
> + return dw_hdmi_phy_power_on(hdmi);
> }
>
> static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
