[PATCH 0/5] drm: ti-tfp410 improvements

[Laurent Pinchart]

Hello,

This small patch series improves the ti-tfp410 driver with three new features,
in patch 2/5 to 5/5. Patch 1/5 has been previously posted by Stefan, and I've
included it here to support patch 5/5 that needs to store other polarity
information in the bridge timings than the sampling edges.

These changes are meant to support the missing tfp410 features currently
implemented in the omapdrm custom tfp410 driver, in order to move to
drm_bridge.

The series is based on top of the "[PATCH v2 0/2] Clarify display info PIXDATA
bus flags" series [1] previously posted on the mailing list.

[1] https://lists.freedesktop.org/archives/dri-devel/2018-December/199204.html

Laurent Pinchart (4):
dt-bindings: display: tfp410: Add bus parameters properties
drm/bridge: ti-tfp410: Set connector type based on DT connector node
drm/bridge: ti-tfp410: Add support for the powerdown GPIO
drm/bridge: ti-tfp410: Report input bus config through bridge timings

Stefan Agner (1):
drm/bridge: use bus flags in bridge timings

.../bindings/display/bridge/ti,tfp410.txt | 24 +++-
drivers/gpu/drm/bridge/dumb-vga-dac.c | 6 +-
drivers/gpu/drm/bridge/ti-tfp410.c | 109 +++++++++++++++++-
include/drm/drm_bridge.h | 12 +-
4 files changed, 131 insertions(+), 20 deletions(-)

[Laurent Pinchart]

From: Stefan Agner <***@agner.ch>

The DRM bus flags convey additional information on pixel data on
the bus. All current available bus flags might be of interest for
a bridge. Remove the sampling_edge field and use bus_flags.

In the case at hand a dumb VGA bridge needs a specific data enable
polarity (DRM_BUS_FLAG_DE_LOW).

Signed-off-by: Stefan Agner <***@agner.ch>
Signed-off-by: Laurent Pinchart <***@ideasonboard.com>
---
Changes since v1:

- Renamed bus_flags to input_bus_flags
---
drivers/gpu/drm/bridge/dumb-vga-dac.c | 6 +++---
include/drm/drm_bridge.h | 12 +++++-------
2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index 7dc14c22f7db..191a4a6e624f 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
*/
static const struct drm_bridge_timings default_dac_timings = {
/* Timing specifications, datasheet page 7 */
- .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+ .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
.setup_time_ps = 500,
.hold_time_ps = 1500,
};
@@ -245,7 +245,7 @@ static const struct drm_bridge_timings default_dac_timings = {
*/
static const struct drm_bridge_timings ti_ths8134_dac_timings = {
/* From timing diagram, datasheet page 9 */
- .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+ .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
/* From datasheet, page 12 */
.setup_time_ps = 3000,
/* I guess this means latched input */
@@ -258,7 +258,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {
*/
static const struct drm_bridge_timings ti_ths8135_dac_timings = {
/* From timing diagram, datasheet page 14 */
- .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+ .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
/* From datasheet, page 16 */
.setup_time_ps = 2000,
.hold_time_ps = 500,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index bf81fb573a5e..27c17706626c 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -244,15 +244,13 @@ struct drm_bridge_funcs {
*/
struct drm_bridge_timings {
/**
- * @sampling_edge:
+ * @input_bus_flags:
*
- * Tells whether the bridge samples the digital input signals from the
- * display engine on the positive or negative edge of the clock. This
- * should use the DRM_BUS_FLAG_PIXDATA_SAMPLE_[POS|NEG]EDGE and
- * DRM_BUS_FLAG_SYNC_SAMPLE_[POS|NEG]EDGE bitwise flags from the DRM
- * connector (bit 2, 3, 6 and 7 valid).
+ * Tells what additional settings for the pixel data on the bus
+ * this bridge requires (like pixel signal polarity). See also
+ * &drm_display_info->bus_flags.
*/
- u32 sampling_edge;
+ u32 input_bus_flags;
/**
* @setup_time_ps:
*

[Laurent Pinchart]

The TI TFP410 is a DVI encoder, not a full HDMI encoder. Its output can
be routed to a DVI-D connector, even if in many cases embedded systems
will use an HDMI connector to carry the DVI signals.

Instead of hardcoding the connector type to HDMI, retrieve the connector
type from its DT node.

Signed-off-by: Laurent Pinchart <***@ideasonboard.com>
---
drivers/gpu/drm/bridge/ti-tfp410.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index c3e32138c6bb..e4280f5af9f5 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -27,6 +27,7 @@
struct tfp410 {
struct drm_bridge bridge;
struct drm_connector connector;
+ unsigned int connector_type;

struct i2c_adapter *ddc;
struct gpio_desc *hpd;
@@ -126,7 +127,7 @@ static int tfp410_attach(struct drm_bridge *bridge)
drm_connector_helper_add(&dvi->connector,
&tfp410_con_helper_funcs);
ret = drm_connector_init(bridge->dev, &dvi->connector,
- &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA);
+ &tfp410_con_funcs, dvi->connector_type);
if (ret) {
dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret);
return ret;
@@ -172,6 +173,11 @@ static int tfp410_get_connector_properties(struct tfp410 *dvi)
if (!connector_node)
return -ENODEV;

+ if (of_device_is_compatible(connector_node, "hdmi-connector"))
+ dvi->connector_type = DRM_MODE_CONNECTOR_HDMIA;
+ else
+ dvi->connector_type = DRM_MODE_CONNECTOR_DVID;
+
dvi->hpd = fwnode_get_named_gpiod(&connector_node->fwnode,
"hpd-gpios", 0, GPIOD_IN, "hpd");
if (IS_ERR(dvi->hpd)) {

[Laurent Pinchart]

The TFP410 supports configurable pixel clock sampling edge and data
de-skew adjustments. The configuration can be set through I2C or
dedicated chip pins.

Report the configuration through the drm_bridge timings. As the
ti-tftp410 driver doesn't support configuring the chip through I2C, we
simply use the default configuration in that case. When the chip is
configured through dedicated pins, we parse the configuration from DT.

Signed-off-by: Laurent Pinchart <***@ideasonboard.com>
---
drivers/gpu/drm/bridge/ti-tfp410.c | 77 ++++++++++++++++++++++++++++--
1 file changed, 74 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index d25d23cfe3f5..48ec647a7526 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -34,6 +34,8 @@ struct tfp410 {
struct delayed_work hpd_work;
struct gpio_desc *powerdown;

+ struct drm_bridge_timings timings;
+
struct device *dev;
};

@@ -180,6 +182,70 @@ static irqreturn_t tfp410_hpd_irq_thread(int irq, void *arg)
return IRQ_HANDLED;
}

+static const struct drm_bridge_timings tfp410_default_timings = {
+ .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE
+ | DRM_BUS_FLAG_DE_HIGH,
+ .setup_time_ps = 1200,
+ .hold_time_ps = 1300,
+};
+
+static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
+{
+ struct drm_bridge_timings *timings = &dvi->timings;
+ struct device_node *ep;
+ u32 pclk_sample = 0;
+ s32 deskew = 0;
+
+ /* Start with defaults. */
+ *timings = tfp410_default_timings;
+
+ if (i2c)
+ /*
+ * In I2C mode timings are configured through the I2C interface.
+ * As the driver doesn't support I2C configuration yet, we just
+ * go with the defaults (BSEL=1, DSEL=1, DKEN=0, EDGE=1).
+ */
+ return 0;
+
+ /*
+ * In non-I2C mode, timings are configured through the BSEL, DSEL, DKEN
+ * and EDGE pins. They are specified in DT through endpoint properties
+ * and vendor-specific properties.
+ */
+ ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 0, 0);
+ if (!ep)
+ return -EINVAL;
+
+ /* Get the sampling edge from the endpoint. */
+ of_property_read_u32(ep, "pclk-sample", &pclk_sample);
+ of_node_put(ep);
+
+ timings->input_bus_flags = DRM_BUS_FLAG_DE_HIGH;
+
+ switch (pclk_sample) {
+ case 0:
+ timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE
+ | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE;
+ break;
+ case 1:
+ timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE
+ | DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Get the setup and hold time from vendor-specific properties. */
+ of_property_read_u32(dvi->dev->of_node, "ti,deskew", (u32 *)&deskew);
+ if (deskew < -4 || deskew > 3)
+ return -EINVAL;
+
+ timings->setup_time_ps = min(0, 1200 - 350 * deskew);
+ timings->hold_time_ps = min(0, 1300 + 350 * deskew);
+
+ return 0;
+}
+
static int tfp410_get_connector_properties(struct tfp410 *dvi)
{
struct device_node *connector_node, *ddc_phandle;
@@ -223,7 +289,7 @@ static int tfp410_get_connector_properties(struct tfp410 *dvi)
return ret;
}

-static int tfp410_init(struct device *dev)
+static int tfp410_init(struct device *dev, bool i2c)
{
struct tfp410 *dvi;
int ret;
@@ -240,8 +306,13 @@ static int tfp410_init(struct device *dev)

dvi->bridge.funcs = &tfp410_bridge_funcs;
dvi->bridge.of_node = dev->of_node;
+ dvi->bridge.timings = &dvi->timings;
dvi->dev = dev;

+ ret = tfp410_parse_timings(dvi, i2c);
+ if (ret)
+ goto fail;
+
ret = tfp410_get_connector_properties(dvi);
if (ret)
goto fail;
@@ -294,7 +365,7 @@ static int tfp410_fini(struct device *dev)

static int tfp410_probe(struct platform_device *pdev)
{
- return tfp410_init(&pdev->dev);
+ return tfp410_init(&pdev->dev, false);
}

static int tfp410_remove(struct platform_device *pdev)
@@ -331,7 +402,7 @@ static int tfp410_i2c_probe(struct i2c_client *client,
return -ENXIO;
}

- return tfp410_init(&client->dev);
+ return tfp410_init(&client->dev, true);
}

static int tfp410_i2c_remove(struct i2c_client *client)

[Laurent Pinchart]

The TFP410 supports configuration of several input bus parameters
through either the I2C port or chip pins. In the latter case, we need to
specify those parameters in DT.

Two new properties are added, ti,deskew to specify the data de-skew
configuration (as set through the DK[3:1] pins), and pclk-sample to
specify the pixel clock sampling edge (as set through the EDGE pin).

Signed-off-by: Laurent Pinchart <***@ideasonboard.com>
---
.../bindings/display/bridge/ti,tfp410.txt | 24 ++++++++++++++-----
1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
index 54d7e31525ec..3f903af93949 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
+++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
@@ -6,15 +6,25 @@ Required properties:

Optional properties:
- powerdown-gpios: power-down gpio
-- reg: I2C address. If and only if present the device node
- should be placed into the i2c controller node where the
- tfp410 i2c is connected to.
+- reg: I2C address. If and only if present the device node should be placed
+ into the I2C controller node where the TFP410 I2C is connected to.
+- ti,deskew: data de-skew in 350ps increments, from -4 to +3, as configured
+ through th DK[3:1] pins. This property shall be present only if the TFP410
+ is not connected through I2C.

Required nodes:
-- Video port 0 for DPI input [1].
-- Video port 1 for DVI output [1].

-[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+This device has two video ports. Their connections are modeled using the OF
+graph bindings specified in [1]. Each port node shall have a single endpoint.
+
+- Port 0 is the DPI input port. Its endpoint subnode shall contain a
+ pclk-sample property and a remote-endpoint property as specified in [1].
+
+- Port 1 is the DVI output port. Its endpoint subnode shall contain a
+ remote-endpoint property is specified in [1].
+
+[1] Documentation/devicetree/bindings/media/video-interfaces.txt
+

Example
-------
@@ -22,6 +32,7 @@ Example
tfp410: ***@0 {
compatible = "ti,tfp410";
powerdown-gpios = <&twl_gpio 2 GPIO_ACTIVE_LOW>;
+ ti,deskew = <4>;

ports {
#address-cells = <1>;
@@ -31,6 +42,7 @@ tfp410: ***@0 {
reg = <0>;

tfp410_in: ***@0 {
+ pclk-sample = <1>;
remote-endpoint = <&dpi_out>;
};
};

[Laurent Pinchart]

The TFP410 has a powerdown pin that can be connected to a GPIO to
control power saving. The DT bindings define a corresponding property,
but the driver doesn't implement support for it. Fix that.

Signed-off-by: Laurent Pinchart <***@ideasonboard.com>
---
drivers/gpu/drm/bridge/ti-tfp410.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index e4280f5af9f5..d25d23cfe3f5 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -32,6 +32,7 @@ struct tfp410 {
struct i2c_adapter *ddc;
struct gpio_desc *hpd;
struct delayed_work hpd_work;
+ struct gpio_desc *powerdown;

struct device *dev;
};
@@ -139,8 +140,24 @@ static int tfp410_attach(struct drm_bridge *bridge)
return 0;
}

+static void tfp410_enable(struct drm_bridge *bridge)
+{
+ struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
+
+ gpiod_set_value_cansleep(dvi->powerdown, 0);
+}
+
+static void tfp410_disable(struct drm_bridge *bridge)
+{
+ struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
+
+ gpiod_set_value_cansleep(dvi->powerdown, 1);
+}
+
static const struct drm_bridge_funcs tfp410_bridge_funcs = {
.attach = tfp410_attach,
+ .enable = tfp410_enable,
+ .disable = tfp410_disable,
};

static void tfp410_hpd_work_func(struct work_struct *work)
@@ -229,6 +246,13 @@ static int tfp410_init(struct device *dev)
if (ret)
goto fail;

+ dvi->powerdown = devm_gpiod_get_optional(dev, "powerdown",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(dvi->powerdown)) {
+ dev_err(dev, "failed to parse powerdown gpio\n");
+ return PTR_ERR(dvi->powerdown);
+ }
+
if (dvi->hpd) {
INIT_DELAYED_WORK(&dvi->hpd_work, tfp410_hpd_work_func);

[Tomi Valkeinen]

Post by Laurent Pinchart
Hello,
This small patch series improves the ti-tfp410 driver with three new features,
in patch 2/5 to 5/5. Patch 1/5 has been previously posted by Stefan, and I've
included it here to support patch 5/5 that needs to store other polarity
information in the bridge timings than the sampling edges.
These changes are meant to support the missing tfp410 features currently
implemented in the omapdrm custom tfp410 driver, in order to move to
drm_bridge.
The series is based on top of the "[PATCH v2 0/2] Clarify display info PIXDATA
bus flags" series [1] previously posted on the mailing list.
[1] https://lists.freedesktop.org/archives/dri-devel/2018-December/199204.html
dt-bindings: display: tfp410: Add bus parameters properties
drm/bridge: ti-tfp410: Set connector type based on DT connector node
drm/bridge: ti-tfp410: Add support for the powerdown GPIO
drm/bridge: ti-tfp410: Report input bus config through bridge timings
drm/bridge: use bus flags in bridge timings
.../bindings/display/bridge/ti,tfp410.txt | 24 +++-
drivers/gpu/drm/bridge/dumb-vga-dac.c | 6 +-
drivers/gpu/drm/bridge/ti-tfp410.c | 109 +++++++++++++++++-
include/drm/drm_bridge.h | 12 +-
4 files changed, 131 insertions(+), 20 deletions(-)

For the series:

Reviewed-by: Tomi Valkeinen <***@ti.com>

Tomi