[PATCH v4 5/9] drm: bridge: dw-hdmi: Fix the PHY power up sequence

[Jose Abreu]

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 at ideasonboard.com>

Reviewed-by: Jose Abreu <joabreu at synopsys.com>

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)