Re: [PATCH v2] drm/bridge: ti-sn65dsi83: Add reversed lvds lanes support

[Wojciech Dubowik <wojciech.dubowik@mt.com>]

On Thu, Jul 09, 2026 at 04:29:44PM +0300, Laurent Pinchart wrote:

Hi Laurent,

> On Thu, Jul 09, 2026 at 02:58:09PM +0200, Wojciech Dubowik wrote:
> > From: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
> > 
> > The chip supports output lvds lanes in two orders, default <1 2 3 4>
> > and <4 3 2 1>. Add parsing of an optional output lvds data-lanes
> > property so we can inform chip that the lanes have been reversed.
> > 
> > Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
> > ---
> > Changes in v2:
> > - Parse existing data-lanes property instead of ading new DT
> >   bindings
> 
> You still need to update the bindings to relax the ordering requirement
> of the data-lanes property.

The data-lanes have been defined for input nodes only. I will add an optinal
binding for output nodes which are handled here.

> 
> Does the device support less than 4 data lanes ? If not, the bindings
> will also need to be updated to reflect that.

There are 4 output lanes only,

Regards,
Wojtek

> 
> > ---
> >  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 50 +++++++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > index 42b451432bbb..4945d4c960c4 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > @@ -148,6 +148,18 @@ enum sn65dsi83_lvds_term {
> >  	OHM_200
> >  };
> >  
> > +enum {
> > +	NORMAL_LANE_MAPPING,
> > +	REVERSE_LANE_MAPPING,
> > +};
> > +
> > +#define DATA_LANES_COUNT	4
> > +
> > +static const int supported_data_lane_mapping[][DATA_LANES_COUNT] = {
> > +	[NORMAL_LANE_MAPPING] = { 1, 2, 3, 4 },
> > +	[REVERSE_LANE_MAPPING] = { 4, 3, 2, 1},
> > +};
> > +
> >  enum sn65dsi83_model {
> >  	MODEL_SN65DSI83,
> >  	MODEL_SN65DSI84,
> > @@ -163,6 +175,7 @@ struct sn65dsi83 {
> >  	struct regulator		*vcc;
> >  	bool				lvds_dual_link;
> >  	bool				lvds_dual_link_even_odd_swap;
> > +	bool				lvds_reverse_lanes_conf[2];
> >  	int				lvds_vod_swing_conf[2];
> >  	int				lvds_term_conf[2];
> >  	int				irq;
> > @@ -644,6 +657,10 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
> >  	regmap_write(ctx->regmap, REG_LVDS_LANE,
> >  		     (ctx->lvds_dual_link_even_odd_swap ?
> >  		      REG_LVDS_LANE_EVEN_ODD_SWAP : 0) |
> > +		     (ctx->lvds_reverse_lanes_conf[CHANNEL_A] ?
> > +		      REG_LVDS_LANE_CHA_REVERSE_LVDS : 0) |
> > +		     (ctx->lvds_reverse_lanes_conf[CHANNEL_B] ?
> > +		      REG_LVDS_LANE_CHB_REVERSE_LVDS : 0) |
> >  		     (ctx->lvds_term_conf[CHANNEL_A] ?
> >  			  REG_LVDS_LANE_CHA_LVDS_TERM : 0) |
> >  		     (ctx->lvds_term_conf[CHANNEL_B] ?
> > @@ -832,10 +849,12 @@ static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
> >  	u32 lvds_vod_swing_clk[2] = { 0, 1000000 };
> >  	/* Set default near end terminataion to 200 Ohm */
> >  	u32 lvds_term = 200;
> > +	u32 data_lanes[DATA_LANES_COUNT];
> >  	int lvds_vod_swing_conf;
> >  	int ret = 0;
> >  	int ret_data;
> >  	int ret_clock;
> > +	int i, j;
> >  
> >  	if (channel == CHANNEL_A)
> >  		endpoint_reg = 2;
> > @@ -854,6 +873,37 @@ static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
> >  		goto exit;
> >  	}
> >  
> > +	ret_data = of_property_read_u32_array(endpoint, "data-lanes", data_lanes,
> > +			ARRAY_SIZE(data_lanes));
> > +	if (ret_data != 0 && ret_data != -EINVAL) {
> > +		ret = ret_data;
> > +		goto exit;
> > +	}
> > +
> > +	if (!ret_data) {
> 
> I wish we could drop this check, but as the data-lanes property is not
> documented as mandatory, I suppose not all device trees use it :-(
> 
> Could you, while at it, make the property mandatory in the bindings, to
> ensure it gets specified in all future device trees (and fix in-tree
> offenders, if any) ?
> 
> > +		for (i = 0; i < ARRAY_SIZE(supported_data_lane_mapping); i++) {
> > +			for (j = 0; j < DATA_LANES_COUNT; j++) {
> > +				if (data_lanes[j] != supported_data_lane_mapping[i][j])
> > +					break;
> > +			}
> > +
> > +			if (j == DATA_LANES_COUNT)
> > +				break;
> > +		}
> > +
> > +		switch (i) {
> > +		case NORMAL_LANE_MAPPING:
> > +			break;
> > +		case REVERSE_LANE_MAPPING:
> > +			ctx->lvds_reverse_lanes_conf[channel] = true;
> > +			break;
> > +		default:
> > +			dev_err(dev, "invalid data lanes mapping\n");
> > +			ret = -EINVAL;
> > +			goto exit;
> > +		}
> > +	}
> > +
> >  	ret_data = of_property_read_u32_array(endpoint, "ti,lvds-vod-swing-data-microvolt",
> >  					lvds_vod_swing_data, ARRAY_SIZE(lvds_vod_swing_data));
> >  	if (ret_data != 0 && ret_data != -EINVAL) {
> 
> -- 
> Regards,
> 
> Laurent Pinchart