[PATCHv2 03/22] drm/bridge: tc358767: fix ansi 8b10b use

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Mar 26, 2019 at 12:31:27PM +0200, Tomi Valkeinen wrote:
> DP always uses ANSI 8B10B encoding. Some monitors (old?) may not have
> the ANSI 8B10B bit set in DPCD, even if it should always be set.

Makes you wonder why the bit is present :-) I've checked the DP v1.0
specification, and even though the bit isn't documented as being always
1, 8B/10B encoding is mandatory, so this should be safe from a DP point
of view.

Without access to the TC358767 datasheet I can't tell what use cases
were intended for disabling 8B/10B encoding. Could it be related to
video sources that already provide X3.230-1994 encoded data ? In any
case this shouldn't be driven by the sink but by the source, so

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Andrey, as this feature was present in the initial driver version that
you authored, do you have more information about its intended use cases
?

> The tc358767 driver currently respects that flag, and turns the encoding
> off if the monitor does not have the bit set, which then results in the
> monitor not working.
> 
> This patch makes the driver to always use ANSI 8B10B encoding, and drops
> the 'coding8b10b' field which is no longer used.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 11a50f7bb4be..163c594fa6ac 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -188,7 +188,6 @@ struct tc_edp_link {
>  	u8			assr;
>  	int			scrambler_dis;
>  	int			spread;
> -	int			coding8b10b;
>  	u8			swing;
>  	u8			preemp;
>  };
> @@ -390,13 +389,10 @@ static u32 tc_srcctrl(struct tc_data *tc)
>  	 * No training pattern, skew lane 1 data by two LSCLK cycles with
>  	 * respect to lane 0 data, AutoCorrect Mode = 0
>  	 */
> -	u32 reg = DP0_SRCCTRL_NOTP | DP0_SRCCTRL_LANESKEW;
> +	u32 reg = DP0_SRCCTRL_NOTP | DP0_SRCCTRL_LANESKEW | DP0_SRCCTRL_EN810B;
>  
>  	if (tc->link.scrambler_dis)
>  		reg |= DP0_SRCCTRL_SCRMBLDIS;	/* Scrambler Disabled */
> -	if (tc->link.coding8b10b)
> -		/* Enable 8/10B Encoder (TxData[19:16] not used) */
> -		reg |= DP0_SRCCTRL_EN810B;
>  	if (tc->link.spread)
>  		reg |= DP0_SRCCTRL_SSCG;	/* Spread Spectrum Enable */
>  	if (tc->link.base.num_lanes == 2)
> @@ -635,7 +631,7 @@ static int tc_get_display_props(struct tc_data *tc)
>  	ret = drm_dp_dpcd_readb(&tc->aux, DP_MAIN_LINK_CHANNEL_CODING, tmp);
>  	if (ret < 0)
>  		goto err_dpcd_read;
> -	tc->link.coding8b10b = tmp[0] & BIT(0);
> +
>  	tc->link.scrambler_dis = 0;
>  	/* read assr */
>  	ret = drm_dp_dpcd_readb(&tc->aux, DP_EDP_CONFIGURATION_SET, tmp);
> @@ -649,7 +645,6 @@ static int tc_get_display_props(struct tc_data *tc)
>  		tc->link.base.num_lanes,
>  		(tc->link.base.capabilities & DP_LINK_CAP_ENHANCED_FRAMING) ?
>  		"enhanced" : "non-enhanced");
> -	dev_dbg(tc->dev, "ANSI 8B/10B: %d\n", tc->link.coding8b10b);
>  	dev_dbg(tc->dev, "Display ASSR: %d, TC358767 ASSR: %d\n",
>  		tc->link.assr, tc->assr);
>  
> @@ -951,7 +946,7 @@ static int tc_main_link_setup(struct tc_data *tc)
>  	/* DOWNSPREAD_CTRL */
>  	tmp[0] = tc->link.spread ? DP_SPREAD_AMP_0_5 : 0x00;
>  	/* MAIN_LINK_CHANNEL_CODING_SET */
> -	tmp[1] =  tc->link.coding8b10b ? DP_SET_ANSI_8B10B : 0x00;
> +	tmp[1] =  DP_SET_ANSI_8B10B;
>  	ret = drm_dp_dpcd_write(aux, DP_DOWNSPREAD_CTRL, tmp, 2);
>  	if (ret < 0)
>  		goto err_dpcd_write;

-- 
Regards,

Laurent Pinchart