On Tue, Dec 03, 2024 at 12:00:53PM +0100, Andrej Picej wrote:
> Add a optional properties to change LVDS output voltage. This should not
> be static as this depends mainly on the connected display voltage
> requirement. We have three properties:
> - "ti,lvds-termination-ohms", which sets near end termination,
> - "ti,lvds-vod-swing-data-microvolt" and
> - "ti,lvds-vod-swing-clock-microvolt" which both set LVDS differential
> output voltage for data and clock lanes. They are defined as an array
> with min and max values. The appropriate bitfiled will be set if
> selected constraints can be met.
>
> If "ti,lvds-termination-ohms" is not defined the default of 200 Ohm near
> end termination will be used. Selecting only one:
> "ti,lvds-vod-swing-data-microvolt" or
> "ti,lvds-vod-swing-clock-microvolt" can be done, but the output voltage
> constraint for only data/clock lanes will be met. Setting both is
> recommended.
>
> Signed-off-by: Andrej Picej <andrej.picej@xxxxxxxxx>
> ---
> Changes in v3:
> - use microvolts for default array values 1000 mV -> 1000000 uV.
> Changes in v2:
> - use datasheet tables to get the proper configuration
> - since major change was done change the authorship to myself
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi83.c | 144 +++++++++++++++++++++++++-
> 1 file changed, 141 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 57a7ed13f996..1a070df5ea12 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -132,6 +132,16 @@
> #define REG_IRQ_STAT_CHA_SOT_BIT_ERR BIT(2)
> #define REG_IRQ_STAT_CHA_PLL_UNLOCK BIT(0)
>
> +enum sn65dsi83_channel {
> + CHANNEL_A,
> + CHANNEL_B
> +};
> +
> +enum sn65dsi83_lvds_term {
> + OHM_100,
> + OHM_200
> +};
> +
> enum sn65dsi83_model {
> MODEL_SN65DSI83,
> MODEL_SN65DSI84,
> @@ -147,6 +157,10 @@ struct sn65dsi83 {
> struct regulator *vcc;
> bool lvds_dual_link;
> bool lvds_dual_link_even_odd_swap;
> + int lvdsA_vod_swing_conf;
> + int lvdsB_vod_swing_conf;
> + int lvdsA_term_conf;
> + int lvdsB_term_conf;
> };
>
> static const struct regmap_range sn65dsi83_readable_ranges[] = {
> @@ -237,6 +251,36 @@ static const struct regmap_config sn65dsi83_regmap_config = {
> .max_register = REG_IRQ_STAT,
> };
>
> +static const int lvds_vod_swing_data_table[2][4][2] = {
> + { /* 100 Ohm */
> + { 180000, 313000 },
> + { 215000, 372000 },
> + { 250000, 430000 },
> + { 290000, 488000 },
> + },
> + { /* 200 Ohm */
> + { 150000, 261000 },
> + { 200000, 346000 },
> + { 250000, 428000 },
> + { 300000, 511000 },
> + },
> +};
> +
> +static const int lvds_vod_swing_clock_table[2][4][2] = {
> + { /* 100 Ohm */
> + { 140000, 244000 },
> + { 168000, 290000 },
> + { 195000, 335000 },
> + { 226000, 381000 },
> + },
> + { /* 200 Ohm */
> + { 117000, 204000 },
> + { 156000, 270000 },
> + { 195000, 334000 },
> + { 234000, 399000 },
> + },
> +};
> +
> static struct sn65dsi83 *bridge_to_sn65dsi83(struct drm_bridge *bridge)
> {
> return container_of(bridge, struct sn65dsi83, bridge);
> @@ -435,12 +479,16 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
> val |= REG_LVDS_FMT_LVDS_LINK_CFG;
>
> regmap_write(ctx->regmap, REG_LVDS_FMT, val);
> - regmap_write(ctx->regmap, REG_LVDS_VCOM, 0x05);
> + regmap_write(ctx->regmap, REG_LVDS_VCOM,
> + REG_LVDS_VCOM_CHA_LVDS_VOD_SWING(ctx->lvdsA_vod_swing_conf) |
> + REG_LVDS_VCOM_CHB_LVDS_VOD_SWING(ctx->lvdsB_vod_swing_conf));
> regmap_write(ctx->regmap, REG_LVDS_LANE,
> (ctx->lvds_dual_link_even_odd_swap ?
> REG_LVDS_LANE_EVEN_ODD_SWAP : 0) |
> - REG_LVDS_LANE_CHA_LVDS_TERM |
> - REG_LVDS_LANE_CHB_LVDS_TERM);
> + (ctx->lvdsA_term_conf ?
> + REG_LVDS_LANE_CHA_LVDS_TERM : 0) |
> + (ctx->lvdsB_term_conf ?
> + REG_LVDS_LANE_CHB_LVDS_TERM : 0));
> regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
>
> le16val = cpu_to_le16(mode->hdisplay);
> @@ -576,10 +624,96 @@ static const struct drm_bridge_funcs sn65dsi83_funcs = {
> .atomic_get_input_bus_fmts = sn65dsi83_atomic_get_input_bus_fmts,
> };
>
> +static int sn65dsi83_select_lvds_vod_swing(struct device *dev,
> + u32 lvds_vod_swing_data[2], u32 lvds_vod_swing_clk[2], u8 lvds_term)
> +{
> + int i;
> +
> + for (i = 0; i <= 3; i++) {
> + if (lvds_vod_swing_data_table[lvds_term][i][0] >= lvds_vod_swing_data[0] &&
> + lvds_vod_swing_data_table[lvds_term][i][1] <= lvds_vod_swing_data[1] &&
> + lvds_vod_swing_clock_table[lvds_term][i][0] >= lvds_vod_swing_clk[0] &&
> + lvds_vod_swing_clock_table[lvds_term][i][1] <= lvds_vod_swing_clk[1])
> + return i;
> + }
> +
> + dev_err(dev, "failed to find appropriate LVDS_VOD_SWING configuration\n");
> + return -EINVAL;
> +}
> +
> +static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
> +{
> + struct device *dev = ctx->dev;
> + struct device_node *endpoint;
> + /* Set so the property can be freely selected if not defined */
> + u32 lvds_vod_swing_data[2] = { 0, 1000000 };
> + u32 lvds_vod_swing_clk[2] = { 0, 1000000 };
> + u32 lvds_term = 200;
> + u8 lvds_term_conf;
> + int endpoint_reg;
> + int lvds_vod_swing_conf;
> + int ret = 0;
> + int ret_data;
> + int ret_clock;
> +
> + if (channel == CHANNEL_A)
> + endpoint_reg = 2;
> + else
> + endpoint_reg = 3;
> +
> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, endpoint_reg, -1);
> + of_property_read_u32(endpoint, "ti,lvds-termination-ohms", &lvds_term);
> +
> + if (lvds_term == 200)
> + lvds_term_conf = OHM_200;
> + else
> + lvds_term_conf = OHM_100;
> +
> + if (channel == CHANNEL_A)
> + ctx->lvdsA_term_conf = lvds_term_conf;
> + else
> + ctx->lvdsB_term_conf = lvds_term_conf;
This sounds like ctx->lvds_term_conf[channel] = ...;
And so on for other properties.
> +
> + ret_data = of_property_read_u32_array(endpoint,
> + "ti,lvds-vod-swing-data-microvolt", lvds_vod_swing_data,
> + ARRAY_SIZE(lvds_vod_swing_data));
> + ret_clock = of_property_read_u32_array(endpoint,
> + "ti,lvds-vod-swing-clock-microvolt", lvds_vod_swing_clk,
> + ARRAY_SIZE(lvds_vod_swing_clk));
> + /* If any of the two properties is defined. */
> + if (!ret_data || !ret_clock) {
I think it's not that easy. We were usually ignoring errors for a
single-value properties, but for the arrays I think we should make a
diffence between the property being not present in DT and the property
being erroneous or not being parsed correctly.
> + lvds_vod_swing_conf = sn65dsi83_select_lvds_vod_swing(dev,
> + lvds_vod_swing_data, lvds_vod_swing_clk,
> + lvds_term_conf);
> + if (lvds_vod_swing_conf < 0) {
> + ret = lvds_vod_swing_conf;
> + goto exit;
> + }
> + if (channel == CHANNEL_A)
> + ctx->lvdsA_vod_swing_conf = lvds_vod_swing_conf;
> + else
> + ctx->lvdsB_vod_swing_conf = lvds_vod_swing_conf;
> + }
> + ret = 0;
> +exit:
> + of_node_put(endpoint);
> + return ret;
> +}
> +
> static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
> {
> struct drm_bridge *panel_bridge;
> struct device *dev = ctx->dev;
> + int ret;
> +
> + ctx->lvdsA_vod_swing_conf = 0x1;
> + ctx->lvdsB_vod_swing_conf = 0x1;
> + ctx->lvdsA_term_conf = 0x1;
> + ctx->lvdsB_term_conf = 0x1;
> +
> + ret = sn65dsi83_parse_lvds_endpoint(ctx, CHANNEL_A);
> + if (ret < 0)
> + return ret;
>
> ctx->lvds_dual_link = false;
> ctx->lvds_dual_link_even_odd_swap = false;
> @@ -587,6 +721,10 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
> struct device_node *port2, *port3;
> int dual_link;
>
> + ret = sn65dsi83_parse_lvds_endpoint(ctx, CHANNEL_B);
> + if (ret < 0)
> + return ret;
> +
> port2 = of_graph_get_port_by_id(dev->of_node, 2);
> port3 = of_graph_get_port_by_id(dev->of_node, 3);
> dual_link = drm_of_lvds_get_dual_link_pixel_order(port2, port3);
> --
> 2.34.1
>
--
With best wishes
Dmitry
