[PATCHv2 14/22] drm/bridge: tc358767: cleanup LT result check

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Mar 26, 2019 at 12:31:38PM +0200, Tomi Valkeinen wrote:
> The driver has a loop after ending link training, where it reads the
> DPCD link status and prints an error if that status is not ok.
> 
> The loop is unnecessary, as far as I can understand from DP specs, so
> let's remove it. We can also print the more specific errors to help
> debugging.

I see in tc_link_training() that the driver checks the channel EQ bits
through the TC358767 DP0_LTSTAT register. Does the chip read the link
status DPCD registers by itself through the AUX link ? If so we could
remove this check completely (unless we don't trust the TC358767 and
want to double-check). If not, where does it get the link status from ?

> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 62 +++++++++++++++++--------------
>  1 file changed, 35 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 700e161015af..220408db82f7 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -968,34 +968,42 @@ static int tc_main_link_enable(struct tc_data *tc)
>  	if (ret < 0)
>  		goto err_dpcd_write;
>  
> -	/* Wait */
> -	timeout = 100;
> -	do {
> -		udelay(1);
> -		/* Read DPCD 0x202-0x207 */
> -		ret = drm_dp_dpcd_read_link_status(aux, tmp + 2);
> -		if (ret < 0)
> -			goto err_dpcd_read;
> -	} while ((--timeout) &&
> -		 !(drm_dp_channel_eq_ok(tmp + 2,  tc->link.base.num_lanes)));
> +	/* Check link status */
> +	ret = drm_dp_dpcd_read_link_status(aux, tmp);
> +	if (ret < 0)
> +		goto err_dpcd_read;
>  
> -	if (timeout == 0) {
> -		/* Read DPCD 0x200-0x201 */
> -		ret = drm_dp_dpcd_read(aux, DP_SINK_COUNT, tmp, 2);
> -		if (ret < 0)
> -			goto err_dpcd_read;
> -		dev_err(dev, "channel(s) EQ not ok\n");
> -		dev_info(dev, "0x0200 SINK_COUNT: 0x%02x\n", tmp[0]);
> -		dev_info(dev, "0x0201 DEVICE_SERVICE_IRQ_VECTOR: 0x%02x\n",
> -			 tmp[1]);
> -		dev_info(dev, "0x0202 LANE0_1_STATUS: 0x%02x\n", tmp[2]);
> -		dev_info(dev, "0x0204 LANE_ALIGN_STATUS_UPDATED: 0x%02x\n",
> -			 tmp[4]);
> -		dev_info(dev, "0x0205 SINK_STATUS: 0x%02x\n", tmp[5]);
> -		dev_info(dev, "0x0206 ADJUST_REQUEST_LANE0_1: 0x%02x\n",
> -			 tmp[6]);
> -
> -		return -EAGAIN;
> +	ret = 0;
> +
> +	value = tmp[0] & DP_CHANNEL_EQ_BITS;
> +
> +	if (value != DP_CHANNEL_EQ_BITS) {
> +		dev_err(tc->dev, "Lane 0 failed: %x\n", value);
> +		ret = -ENODEV;
> +	}
> +
> +	if (tc->link.base.num_lanes == 2) {
> +		value = (tmp[0] >> 4) & DP_CHANNEL_EQ_BITS;
> +
> +		if (value != DP_CHANNEL_EQ_BITS) {
> +			dev_err(tc->dev, "Lane 1 failed: %x\n", value);
> +			ret = -ENODEV;
> +		}
> +
> +		if (!(tmp[2] & DP_INTERLANE_ALIGN_DONE)) {
> +			dev_err(tc->dev, "Interlane align failed\n");
> +			ret = -ENODEV;
> +		}
> +	}
> +
> +	if (ret) {
> +		dev_err(dev, "0x0202 LANE0_1_STATUS:            0x%02x\n", tmp[0]);
> +		dev_err(dev, "0x0203 LANE2_3_STATUS             0x%02x\n", tmp[1]);
> +		dev_err(dev, "0x0204 LANE_ALIGN_STATUS_UPDATED: 0x%02x\n", tmp[2]);
> +		dev_err(dev, "0x0205 SINK_STATUS:               0x%02x\n", tmp[3]);
> +		dev_err(dev, "0x0206 ADJUST_REQUEST_LANE0_1:    0x%02x\n", tmp[4]);
> +		dev_err(dev, "0x0207 ADJUST_REQUEST_LANE2_3:    0x%02x\n", tmp[5]);
> +		goto err;
>  	}
>  
>  	return 0;

-- 
Regards,

Laurent Pinchart