Hi,
the purpose of this small series is to enable the support for the reverse lane feature and to add support for reset controllers which can drive the enable pin.
Regards, Marco
Marco Felsch (6): drm/bridge: ti-sn65dsi83: make lvds lane register setup more readable dt-bindings: drm/bridge: ti-sn65dsi83: add documentation for reverse lvds lanes drm/bridge: ti-sn65dsi83: add support to swap the LVDS data lanes drm/bridge: ti-sn65dsi83: make use of dev_err_probe dt-bindings: drm/bridge: ti-sn65dsi83: Add reset controller documentation drm/bridge: ti-sn65dsi83: add support for a external reset controller
.../bindings/display/bridge/ti,sn65dsi83.yaml | 64 ++++++++++++- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 89 +++++++++++++++++-- 2 files changed, 145 insertions(+), 8 deletions(-)
No functional change. Just reuse the already existing val variable to setup the register. This is in preparation for adding the new feature to reverse the CHA/CHB lane orders. Without this change this call gets very unreadable.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 2831f0813c3a..112fea004c8e 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -437,11 +437,12 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
regmap_write(ctx->regmap, REG_LVDS_FMT, val); regmap_write(ctx->regmap, REG_LVDS_VCOM, 0x05); - 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); + + val = REG_LVDS_LANE_CHA_LVDS_TERM | REG_LVDS_LANE_CHB_LVDS_TERM; + if (ctx->lvds_dual_link_even_odd_swap) + val |= REG_LVDS_LANE_EVEN_ODD_SWAP; + + regmap_write(ctx->regmap, REG_LVDS_LANE, val); regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
le16val = cpu_to_le16(mode->hdisplay);
The TI converter chip can swap the LVDS data lanes in a pre-defined manner. This can be useful to improve the layout characteristic.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- .../bindings/display/bridge/ti,sn65dsi83.yaml | 58 ++++++++++++++++++- 1 file changed, 56 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index 48a97bb3e2e0..7306b9874dc3 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml @@ -80,13 +80,67 @@ properties: - const: 4
port@2: - $ref: /schemas/graph.yaml#/properties/port + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false description: Video port for LVDS Channel-A output (panel or bridge).
+ properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: + description: | + Array of physical LVDS data lane indexes in reverse or normal + order. Specify it in reverse order to enable the bridge CHA + reverse ordering. If not specified normal order is used. + items: + oneOf: + # reverse order + - items: + - const: 4 + - const: 3 + - const: 2 + - const: 1 + # normal order + - items: + - const: 1 + - const: 2 + - const: 3 + - const: 4 + port@3: - $ref: /schemas/graph.yaml#/properties/port + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false description: Video port for LVDS Channel-B output (panel or bridge).
+ properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: + description: | + Array of physical LVDS data lane indexes in reverse or normal + order. Specify it in reverse order to enable the bridge CHB + reverse ordering. If not specified normal order is used. + items: + oneOf: + # reverse order + - items: + - const: 4 + - const: 3 + - const: 2 + - const: 1 + # normal order + - items: + - const: 1 + - const: 2 + - const: 3 + - const: 4 + required: - port@0 - port@2
On Mon, May 30, 2022 at 05:05:45PM +0200, Marco Felsch wrote:
The TI converter chip can swap the LVDS data lanes in a pre-defined manner. This can be useful to improve the layout characteristic.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de
.../bindings/display/bridge/ti,sn65dsi83.yaml | 58 ++++++++++++++++++- 1 file changed, 56 insertions(+), 2 deletions(-)
Reviewed-by: Rob Herring robh@kernel.org
The chip can swap the LVDS channel A/B data lanes e.g. to improve the layout characteristic. This commit adds the feature so the system integrator can specify it within the device-tree.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 64 +++++++++++++++++++++++++++ 1 file changed, 64 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 112fea004c8e..baf94b2b78a1 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -32,6 +32,7 @@ #include <linux/module.h> #include <linux/of_device.h> #include <linux/of_graph.h> +#include <linux/property.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h>
@@ -148,6 +149,8 @@ struct sn65dsi83 { int dsi_lanes; bool lvds_dual_link; bool lvds_dual_link_even_odd_swap; + bool lvds_reverse_cha; + bool lvds_reverse_chb; };
static const struct regmap_range sn65dsi83_readable_ranges[] = { @@ -441,6 +444,10 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge, val = REG_LVDS_LANE_CHA_LVDS_TERM | REG_LVDS_LANE_CHB_LVDS_TERM; if (ctx->lvds_dual_link_even_odd_swap) val |= REG_LVDS_LANE_EVEN_ODD_SWAP; + if (ctx->lvds_reverse_cha) + val |= REG_LVDS_LANE_CHA_REVERSE_LVDS; + if (ctx->lvds_reverse_chb) + val |= REG_LVDS_LANE_CHB_REVERSE_LVDS;
regmap_write(ctx->regmap, REG_LVDS_LANE, val); regmap_write(ctx->regmap, REG_LVDS_CM, 0x00); @@ -566,6 +573,47 @@ static const struct drm_bridge_funcs sn65dsi83_funcs = { .atomic_get_input_bus_fmts = sn65dsi83_atomic_get_input_bus_fmts, };
+static int sn65dsi83_parse_lvds_lane_order(struct sn65dsi83 *ctx, unsigned char port) +{ + struct device *dev = ctx->dev; + struct device_node *ep; + int lvds_lanes; + int ret = 0; + + if (port < 2 || port > 3) + return -EINVAL; + + ep = of_graph_get_endpoint_by_regs(dev->of_node, port, 0); + lvds_lanes = of_property_count_u32_elems(ep, "data-lanes"); + if (lvds_lanes == 4) { + u32 lane_assignments[] = { 1, 2, 3, 4 }; + + of_property_read_u32_array(ep, "data-lanes", lane_assignments, + lvds_lanes); + if (lane_assignments[0] == 4 && + lane_assignments[1] == 3 && + lane_assignments[2] == 2 && + lane_assignments[3] == 1) { + if (port == 2) + ctx->lvds_reverse_cha = true; + else + ctx->lvds_reverse_chb = true; + } else if (lane_assignments[0] != 1 || + lane_assignments[1] != 2 || + lane_assignments[2] != 3 || + lane_assignments[3] != 4) { + dev_err(dev, "Unsupported LVDS lane order\n"); + ret = -EINVAL; + } + } else if (lvds_lanes > 0) { + dev_err(dev, "All 4 LVDS data-lanes must be specified\n"); + ret = -EINVAL; + } + of_node_put(ep); + + return ret; +} + static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model) { struct drm_bridge *panel_bridge; @@ -610,6 +658,22 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model) } }
+ /* + * Todo: + * Check if the reverse lane setup is working in dual-link as well. + */ + if (!ctx->lvds_dual_link) { + ret = sn65dsi83_parse_lvds_lane_order(ctx, 2); + if (ret) + return ret; + + if (model != MODEL_SN65DSI83) { + ret = sn65dsi83_parse_lvds_lane_order(ctx, 3); + if (ret) + return ret; + } + } + panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 2, 0); if (IS_ERR(panel_bridge)) { ret = PTR_ERR(panel_bridge);
Use the new helper to improve the debug capabilities.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index baf94b2b78a1..3c1dc16985b5 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -757,7 +757,8 @@ static int sn65dsi83_probe(struct i2c_client *client, ctx->enable_gpio = devm_gpiod_get_optional(ctx->dev, "enable", GPIOD_OUT_LOW); if (IS_ERR(ctx->enable_gpio)) - return PTR_ERR(ctx->enable_gpio); + return dev_err_probe(ctx->dev, PTR_ERR(ctx->enable_gpio), + "Failed to get GPIO\n");
usleep_range(10000, 11000);
The bridge device can now also be enabled/disabled by an external reset controller. So the device now supports either enable/disable by simple GPIO or by an Reset-Controller.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index 7306b9874dc3..eff8360c184e 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml @@ -35,6 +35,12 @@ properties: vcc-supply: description: A 1.8V power supply (see regulator/regulator.yaml).
+ resets: + maxItems: 1 + description: | + Reset specifier for bridge_en pin. This is required only if the brdige_en + pin is connected to a reset controller. + ports: $ref: /schemas/graph.yaml#/properties/ports
Hi Marco,
Am Montag, 30. Mai 2022, 17:05:48 CEST schrieb Marco Felsch:
The bridge device can now also be enabled/disabled by an external reset controller. So the device now supports either enable/disable by simple GPIO or by an Reset-Controller.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de
.../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index 7306b9874dc3..eff8360c184e 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml @@ -35,6 +35,12 @@ properties: vcc-supply: description: A 1.8V power supply (see regulator/regulator.yaml).
- resets:
- maxItems: 1
- description: |
Reset specifier for bridge_en pin. This is required only if thebrdige_en + pin is connected to a reset controller.
- ports: $ref: /schemas/graph.yaml#/properties/ports
Maybe it should be added here, that 'resets' is an alternative to 'enable- gpios' as both are essentially controlling the same pin from the bridge.
Best regards Alexander
Hi Alexander,
On 22-05-31, Alexander Stein wrote:
Hi Marco,
Am Montag, 30. Mai 2022, 17:05:48 CEST schrieb Marco Felsch:
The bridge device can now also be enabled/disabled by an external reset controller. So the device now supports either enable/disable by simple GPIO or by an Reset-Controller.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de
.../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index 7306b9874dc3..eff8360c184e 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml @@ -35,6 +35,12 @@ properties: vcc-supply: description: A 1.8V power supply (see regulator/regulator.yaml).
- resets:
- maxItems: 1
- description: |
Reset specifier for bridge_en pin. This is required only if thebrdige_en + pin is connected to a reset controller.
- ports: $ref: /schemas/graph.yaml#/properties/ports
Maybe it should be added here, that 'resets' is an alternative to 'enable- gpios' as both are essentially controlling the same pin from the bridge.
I mention the bridge_en pin two times :) But I could also add that this is an alternative to the enable-gpios. Maybe it even more clear than.
Regards, Marco
Best regards Alexander
The bridge chip has an enable gpio which of course can enable/disable the bridge. Most the time this gpio is connected directly to the host but sometimes it is connected to a reset controller chip and the host controlls the reset controller chip instead. This commit adds the support to handle that.
Therefore we need either the reset controller or a gpio to be present and valid. The behaviour is changed in that way that a gpio or a reset controller have to be successfully requested else the driver probe fails, like the current behaviour.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 3c1dc16985b5..7b232a4f8bcb 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -35,6 +35,7 @@ #include <linux/property.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> +#include <linux/reset.h>
#include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> @@ -146,6 +147,7 @@ struct sn65dsi83 { struct drm_bridge *panel_bridge; struct gpio_desc *enable_gpio; struct regulator *vcc; + struct reset_control *reset; int dsi_lanes; bool lvds_dual_link; bool lvds_dual_link_even_odd_swap; @@ -350,6 +352,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
/* Deassert reset */ gpiod_set_value(ctx->enable_gpio, 1); + reset_control_deassert(ctx->reset); usleep_range(1000, 1100);
/* Get the LVDS format from the bridge state. */ @@ -511,6 +514,7 @@ static void sn65dsi83_atomic_disable(struct drm_bridge *bridge,
/* Put the chip in reset, pull EN line low, and assure 10ms reset low timing. */ gpiod_set_value(ctx->enable_gpio, 0); + reset_control_assert(ctx->reset); usleep_range(10000, 11000);
ret = regulator_disable(ctx->vcc); @@ -760,6 +764,13 @@ static int sn65dsi83_probe(struct i2c_client *client, return dev_err_probe(ctx->dev, PTR_ERR(ctx->enable_gpio), "Failed to get GPIO\n");
+ /* Or use a external reset chip to do so */ + ctx->reset = devm_reset_control_get_optional(ctx->dev, NULL); + if (IS_ERR(ctx->reset)) + return dev_err_probe(ctx->dev, PTR_ERR(ctx->reset), + "Failed to get reset\n"); + reset_control_assert(ctx->reset); + usleep_range(10000, 11000);
ret = sn65dsi83_parse_dt(ctx, model);
dri-devel@lists.freedesktop.org
-
Alexander Stein -
Marco Felsch -
Rob Herring