Re: [PATCH v3 7/9] drm/bridge: ti-sn65dsi86: Group DP link training bits in a function

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Douglas Anderson <dianders@chromium.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	robdclark@chromium.org, linux-arm-msm@vger.kernel.org,
	seanpaul@chromium.org, Jeffrey Hugo <jeffrey.l.hugo@gmail.com>,
	Daniel Vetter <daniel@ffwll.ch>, Rob Clark <robdclark@gmail.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	David Airlie <airlied@linux.ie>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH v3 7/9] drm/bridge: ti-sn65dsi86: Group DP link training bits in a function
Date: Mon, 3 Feb 2020 15:39:01 -0800	[thread overview]
Message-ID: <20200203233901.GG311651@builder> (raw)
In-Reply-To: <20191218143416.v3.7.I1fc75ad11db9048ef08cfe1ab7322753d9a219c7@changeid>

On Wed 18 Dec 14:35 PST 2019, Douglas Anderson wrote:

> We'll re-organize the ti_sn_bridge_enable() function a bit to group
> together all the parts relating to link training and split them into a
> sub-function.  This is not intended to have any functional change and
> is in preparation for trying link training several times at different
> rates.  One small side effect here is that if link training fails
> we'll now leave the DP PLL disabled, but that seems like a sane thing
> to do.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Tested-by: Rob Clark <robdclark@gmail.com>
> Reviewed-by: Rob Clark <robdclark@gmail.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> ---
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 86 ++++++++++++++++-----------
>  1 file changed, 52 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index d5990a0947b9..48fb4dc72e1c 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -530,6 +530,46 @@ static unsigned int ti_sn_get_max_lanes(struct ti_sn_bridge *pdata)
>  	return data & DP_LANE_COUNT_MASK;
>  }
>  
> +static int ti_sn_link_training(struct ti_sn_bridge *pdata)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	/* set dp clk frequency value */
> +	ti_sn_bridge_set_dp_rate(pdata);
> +
> +	/* enable DP PLL */
> +	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1);
> +
> +	ret = regmap_read_poll_timeout(pdata->regmap, SN_DPPLL_SRC_REG, val,
> +				       val & DPPLL_SRC_DP_PLL_LOCK, 1000,
> +				       50 * 1000);
> +	if (ret) {
> +		DRM_ERROR("DP_PLL_LOCK polling failed (%d)\n", ret);
> +		goto exit;
> +	}
> +
> +	/* Semi auto link training mode */
> +	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> +	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> +				       val == ML_TX_MAIN_LINK_OFF ||
> +				       val == ML_TX_NORMAL_MODE, 1000,
> +				       500 * 1000);
> +	if (ret) {
> +		DRM_ERROR("Training complete polling failed (%d)\n", ret);
> +	} else if (val == ML_TX_MAIN_LINK_OFF) {
> +		DRM_ERROR("Link training failed, link is off\n");
> +		ret = -EIO;
> +	}
> +
> +exit:
> +	/* Disable the PLL if we failed */
> +	if (ret)
> +		regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 0);
> +
> +	return ret;
> +}
> +
>  static void ti_sn_bridge_enable(struct drm_bridge *bridge)
>  {
>  	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
> @@ -555,29 +595,8 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
>  	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
>  			   CHA_DSI_LANES_MASK, val);
>  
> -	/* Set the DP output format (18 bpp or 24 bpp) */
> -	val = (ti_sn_bridge_get_bpp(pdata) == 18) ? BPP_18_RGB : 0;
> -	regmap_update_bits(pdata->regmap, SN_DATA_FORMAT_REG, BPP_18_RGB, val);
> -
> -	/* DP lane config */
> -	val = DP_NUM_LANES(min(pdata->dp_lanes, 3));
> -	regmap_update_bits(pdata->regmap, SN_SSC_CONFIG_REG, DP_NUM_LANES_MASK,
> -			   val);
> -
> -	/* set dsi/dp clk frequency value */
> +	/* set dsi clk frequency value */
>  	ti_sn_bridge_set_dsi_rate(pdata);
> -	ti_sn_bridge_set_dp_rate(pdata);
> -
> -	/* enable DP PLL */
> -	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1);
> -
> -	ret = regmap_read_poll_timeout(pdata->regmap, SN_DPPLL_SRC_REG, val,
> -				       val & DPPLL_SRC_DP_PLL_LOCK, 1000,
> -				       50 * 1000);
> -	if (ret) {
> -		DRM_ERROR("DP_PLL_LOCK polling failed (%d)\n", ret);
> -		return;
> -	}
>  
>  	/**
>  	 * The SN65DSI86 only supports ASSR Display Authentication method and
> @@ -588,19 +607,18 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
>  	drm_dp_dpcd_writeb(&pdata->aux, DP_EDP_CONFIGURATION_SET,
>  			   DP_ALTERNATE_SCRAMBLER_RESET_ENABLE);
>  
> -	/* Semi auto link training mode */
> -	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> -	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> -				       val == ML_TX_MAIN_LINK_OFF ||
> -				       val == ML_TX_NORMAL_MODE, 1000,
> -				       500 * 1000);
> -	if (ret) {
> -		DRM_ERROR("Training complete polling failed (%d)\n", ret);
> -		return;
> -	} else if (val == ML_TX_MAIN_LINK_OFF) {
> -		DRM_ERROR("Link training failed, link is off\n");
> +	/* Set the DP output format (18 bpp or 24 bpp) */
> +	val = (ti_sn_bridge_get_bpp(pdata) == 18) ? BPP_18_RGB : 0;
> +	regmap_update_bits(pdata->regmap, SN_DATA_FORMAT_REG, BPP_18_RGB, val);
> +
> +	/* DP lane config */
> +	val = DP_NUM_LANES(min(pdata->dp_lanes, 3));
> +	regmap_update_bits(pdata->regmap, SN_SSC_CONFIG_REG, DP_NUM_LANES_MASK,
> +			   val);
> +
> +	ret = ti_sn_link_training(pdata);
> +	if (ret)
>  		return;
> -	}
>  
>  	/* config video parameters */
>  	ti_sn_bridge_set_video_timings(pdata);
> -- 
> 2.24.1.735.g03f4e72817-goog
> 