The TFP410 driver does not support I2C. As such, the device remains in
Power Down if the I2C is enabled by the bootstrap pins.
Add basic support for the I2C interface, and provide support to take
the device out of power down when enabled. Also read the bootstrap mode
pins via the CTL_1_MODE register when using the I2C bus.
Also allow polling device to support hdmi/dvi hotplug detection.
To: Andrzej Hajda <[email protected]>
To: Neil Armstrong <[email protected]>
To: Robert Foss <[email protected]>
To: Laurent Pinchart <[email protected]>
To: Jonas Karlman <[email protected]>
To: Jernej Skrabec <[email protected]>
To: David Airlie <[email protected]>
To: Daniel Vetter <[email protected]>
To: Rob Herring <[email protected]>
To: Krzysztof Kozlowski <[email protected]>
To: Tomi Valkeinen <[email protected]>
To: Jyri Sarha <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Michael Williamson <[email protected]>
Cc: Bob Duke <[email protected]>
Signed-off-by: Jonathan Cormier <[email protected]>
---
Jonathan Cormier (1):
dt-bindings: display: bridge: tfp410: Add tfp410 i2c example
Michael Williamson (3):
DRM: BRIDGE: TFP410: Support basic I2C interface
DRM: BRIDGE: TFP410: Fix logic to configured polled HPD
DRM: BRIDGE: TFP410: If connected, use I2C for polled HPD status.
.../bindings/display/bridge/ti,tfp410.yaml | 42 ++++++++
drivers/gpu/drm/bridge/ti-tfp410.c | 110 +++++++++++++++------
2 files changed, 124 insertions(+), 28 deletions(-)
---
base-commit: 93f875a8526a291005e7f38478079526c843cbec
change-id: 20230125-tfp410_i2c-3b270b0bf3e0
Best regards,
--
Jonathan Cormier <[email protected]>
Add a i2c example with HDMI connector
Signed-off-by: Jonathan Cormier <[email protected]>
---
.../bindings/display/bridge/ti,tfp410.yaml | 42 ++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
index 4c5dd8ec2951..456214f14b47 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
@@ -116,4 +116,46 @@ examples:
};
};
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hdmi_encoder: tfp410@38 {
+ compatible = "ti,tfp410";
+ reg = <0x38>;
+
+ ports {
+ address-cells = <1>;
+ size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ tfp410_in: endpoint {
+ remote-endpoint = <&dpi1_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ tfp410_out: endpoint {
+ remote-endpoint = <&hdmi_connector_in>;
+ };
+ };
+ };
+ };
+ };
+
+ hdmi: hdmi_connector {
+ compatible = "hdmi-connector";
+ label = "hdmi";
+ type = "a";
+ ddc-i2c-bus = <&i2c1>;
+ port {
+ hdmi_connector_in: endpoint {
+ remote-endpoint = <&tfp410_out>;
+ };
+ };
+ };
+
...
--
2.25.1
From: Michael Williamson <[email protected]>
The TFP410 driver does not support I2C. As such, the device remains in
Power Down if the I2C is enabled by the bootstrap pins.
Add basic support for the I2C interface, and provide support to take
the device out of power down when enabled. Also read the bootstrap mode
pins via the CTL_1_MODE register when using the I2C bus.
Signed-off-by: Michael Williamson <[email protected]>
Signed-off-by: Jonathan Cormier <[email protected]>
---
drivers/gpu/drm/bridge/ti-tfp410.c | 95 +++++++++++++++++++++++++++-----------
1 file changed, 68 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index b9635abbad16..323a6d9ed188 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -6,6 +6,7 @@
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
+#include <linux/regmap.h>
#include <linux/media-bus-format.h>
#include <linux/module.h>
#include <linux/of_graph.h>
@@ -21,6 +22,20 @@
#define HOTPLUG_DEBOUNCE_MS 1100
+#define TFP410_REG_CTL_1_MODE 0x08
+#define TFP410_BIT_PD BIT(0)
+#define TFP410_BIT_EDGE BIT(1)
+#define TFP410_BIT_BSEL BIT(2)
+#define TFP410_BIT_DSEL BIT(3)
+
+static const struct regmap_config tfp410_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = 0xff,
+ .cache_type = REGCACHE_NONE,
+};
+
struct tfp410 {
struct drm_bridge bridge;
struct drm_connector connector;
@@ -33,6 +48,8 @@ struct tfp410 {
struct drm_bridge *next_bridge;
struct device *dev;
+ struct i2c_client *i2c;
+ struct regmap *regmap;
};
static inline struct tfp410 *
@@ -183,6 +200,9 @@ static void tfp410_enable(struct drm_bridge *bridge)
{
struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
+ if (dvi->i2c)
+ regmap_set_bits(dvi->regmap, TFP410_REG_CTL_1_MODE, TFP410_BIT_PD);
+
gpiod_set_value_cansleep(dvi->powerdown, 0);
}
@@ -190,6 +210,9 @@ static void tfp410_disable(struct drm_bridge *bridge)
{
struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
+ if (dvi->i2c)
+ regmap_clear_bits(dvi->regmap, TFP410_REG_CTL_1_MODE, TFP410_BIT_PD);
+
gpiod_set_value_cansleep(dvi->powerdown, 1);
}
@@ -221,38 +244,48 @@ static const struct drm_bridge_timings tfp410_default_timings = {
.hold_time_ps = 1300,
};
-static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
+static int tfp410_parse_timings(struct tfp410 *dvi)
{
struct drm_bridge_timings *timings = &dvi->timings;
struct device_node *ep;
u32 pclk_sample = 0;
u32 bus_width = 24;
u32 deskew = 0;
+ unsigned int val = 0;
+ int ret = 0;
/* Start with defaults. */
*timings = tfp410_default_timings;
- if (i2c)
+ if (dvi->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).
+ * For now, assume settings are latched from pins on reset / power up.
+ * Should add options to optionally set them out of DT properties.
*/
- 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_property_read_u32(ep, "bus-width", &bus_width);
- of_node_put(ep);
+ ret = regmap_read(dvi->regmap, TFP410_REG_CTL_1_MODE, &val);
+ if (ret) {
+ dev_err(dvi->dev, "Read failed on CTL_1_MODE\n");
+ return ret;
+ }
+ pclk_sample = (val & TFP410_BIT_EDGE) ? 1 : 0;
+ bus_width = (val & TFP410_BIT_BSEL) ? 24 : 12;
+ dev_dbg(dvi->dev, "(0x%02X) : detected %d bus width, %s edge sampling\n",
+ val, bus_width, pclk_sample ? "positive" : "negative");
+ } else {
+ /*
+ * 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_property_read_u32(ep, "bus-width", &bus_width);
+ of_node_put(ep);
+ }
timings->input_bus_flags = DRM_BUS_FLAG_DE_HIGH;
@@ -291,7 +324,7 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
return 0;
}
-static int tfp410_init(struct device *dev, bool i2c)
+static int tfp410_init(struct device *dev, struct i2c_client *i2c)
{
struct device_node *node;
struct tfp410 *dvi;
@@ -313,15 +346,24 @@ static int tfp410_init(struct device *dev, bool i2c)
dvi->bridge.of_node = dev->of_node;
dvi->bridge.timings = &dvi->timings;
dvi->bridge.type = DRM_MODE_CONNECTOR_DVID;
+ dvi->i2c = i2c;
+
+ if (i2c) {
+ dvi->regmap = devm_regmap_init_i2c(i2c, &tfp410_regmap_config);
+ if (IS_ERR(dvi->regmap))
+ return PTR_ERR(dvi->regmap);
+ }
- ret = tfp410_parse_timings(dvi, i2c);
+ ret = tfp410_parse_timings(dvi);
if (ret)
return ret;
/* Get the next bridge, connected to port@1. */
node = of_graph_get_remote_node(dev->of_node, 1, -1);
- if (!node)
+ if (!node) {
+ dev_err(dev, "Could not find remote node\n");
return -ENODEV;
+ }
dvi->next_bridge = of_drm_find_bridge(node);
of_node_put(node);
@@ -352,7 +394,7 @@ static void tfp410_fini(struct device *dev)
static int tfp410_probe(struct platform_device *pdev)
{
- return tfp410_init(&pdev->dev, false);
+ return tfp410_init(&pdev->dev, NULL);
}
static int tfp410_remove(struct platform_device *pdev)
@@ -378,7 +420,6 @@ static struct platform_driver tfp410_platform_driver = {
};
#if IS_ENABLED(CONFIG_I2C)
-/* There is currently no i2c functionality. */
static int tfp410_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -391,7 +432,7 @@ static int tfp410_i2c_probe(struct i2c_client *client,
return -ENXIO;
}
- return tfp410_init(&client->dev, true);
+ return tfp410_init(&client->dev, client);
}
static void tfp410_i2c_remove(struct i2c_client *client)
@@ -408,7 +449,7 @@ MODULE_DEVICE_TABLE(i2c, tfp410_i2c_ids);
static struct i2c_driver tfp410_i2c_driver = {
.driver = {
.name = "tfp410",
- .of_match_table = of_match_ptr(tfp410_match),
+ .of_match_table = tfp410_match,
},
.id_table = tfp410_i2c_ids,
.probe = tfp410_i2c_probe,
--
2.25.1
From: Michael Williamson <[email protected]>
The logic to configure polling (vs async/irq notification) of hot-plug
events was not correct. If the connected bridge requires polling,
then inform the upstream bridge we also require polling.
Signed-off-by: Michael Williamson <[email protected]>
Signed-off-by: Jonathan Cormier <[email protected]>
---
drivers/gpu/drm/bridge/ti-tfp410.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index 323a6d9ed188..837e1f81a0ff 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -155,7 +155,7 @@ static int tfp410_attach(struct drm_bridge *bridge,
return -ENODEV;
}
- if (dvi->next_bridge->ops & DRM_BRIDGE_OP_DETECT)
+ if (dvi->next_bridge->ops & DRM_BRIDGE_OP_HPD)
dvi->connector.polled = DRM_CONNECTOR_POLL_HPD;
else
dvi->connector.polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
--
2.25.1
From: Michael Williamson <[email protected]>
If the I2C bus is connected on the TFP410, then use the register
status bit to determine connection state. This is needed, in particular,
for polling the state when the Hot Plug detect is not connected to
a controlling CPU via GPIO/IRQ lane.
Signed-off-by: Michael Williamson <[email protected]>
Signed-off-by: Jonathan Cormier <[email protected]>
---
drivers/gpu/drm/bridge/ti-tfp410.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index 837e1f81a0ff..ac216eaec3c8 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -28,6 +28,9 @@
#define TFP410_BIT_BSEL BIT(2)
#define TFP410_BIT_DSEL BIT(3)
+#define TFP410_REG_CTL_2_MODE 0x09
+#define TFP410_BIT_HTPLG BIT(1)
+
static const struct regmap_config tfp410_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
@@ -105,6 +108,16 @@ static enum drm_connector_status
tfp410_connector_detect(struct drm_connector *connector, bool force)
{
struct tfp410 *dvi = drm_connector_to_tfp410(connector);
+ u32 val;
+ unsigned int ret;
+
+ if (dvi->i2c) {
+ ret = regmap_test_bits(dvi->regmap, TFP410_REG_CTL_2_MODE, TFP410_BIT_HTPLG);
+ if (ret < 0)
+ dev_err(dvi->dev, "%s failed to read HTPLG bit : %d\n", __func__, ret);
+ else
+ return ret ? connector_status_connected : connector_status_disconnected;
+ }
return drm_bridge_detect(dvi->next_bridge);
}
--
2.25.1
Hi Jonathan,
Thank you for the patch.
On Wed, Jan 25, 2023 at 04:09:09PM -0500, Jonathan Cormier wrote:
> Add a i2c example with HDMI connector
>
> Signed-off-by: Jonathan Cormier <[email protected]>
> ---
> .../bindings/display/bridge/ti,tfp410.yaml | 42 ++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> index 4c5dd8ec2951..456214f14b47 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> @@ -116,4 +116,46 @@ examples:
> };
> };
>
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
Please use 4 spaces for indentation, as in the other example.
> +
> + hdmi_encoder: tfp410@38 {
> + compatible = "ti,tfp410";
> + reg = <0x38>;
> +
> + ports {
> + address-cells = <1>;
> + size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + tfp410_in: endpoint {
> + remote-endpoint = <&dpi1_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + tfp410_out: endpoint {
> + remote-endpoint = <&hdmi_connector_in>;
> + };
> + };
> + };
> + };
> + };
> +
> + hdmi: hdmi_connector {
> + compatible = "hdmi-connector";
> + label = "hdmi";
> + type = "a";
> + ddc-i2c-bus = <&i2c1>;
> + port {
> + hdmi_connector_in: endpoint {
> + remote-endpoint = <&tfp410_out>;
> + };
> + };
> + };
> +
You can drop the hdmi connector, the example will still validate.
> ...
--
Regards,
Laurent Pinchart
On Wed, Jan 25, 2023 at 4:24 PM Laurent Pinchart
<[email protected]> wrote:
>
> Hi Jonathan,
>
> Thank you for the patch.
>
> On Wed, Jan 25, 2023 at 04:09:09PM -0500, Jonathan Cormier wrote:
> > Add a i2c example with HDMI connector
> >
> > Signed-off-by: Jonathan Cormier <[email protected]>
> > ---
> > .../bindings/display/bridge/ti,tfp410.yaml | 42 ++++++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> > index 4c5dd8ec2951..456214f14b47 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> > @@ -116,4 +116,46 @@ examples:
> > };
> > };
> >
> > + - |
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
>
> Please use 4 spaces for indentation, as in the other example.
Will do, the whole file is 2 space indents. I didn't notice the
examples switch to 4 spaces.
>
> > +
> > + hdmi_encoder: tfp410@38 {
> > + compatible = "ti,tfp410";
> > + reg = <0x38>;
> > +
> > + ports {
> > + address-cells = <1>;
> > + size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + tfp410_in: endpoint {
> > + remote-endpoint = <&dpi1_out>;
> > + };
> > + };
> > +
> > + port@1 {
> > + reg = <1>;
> > + tfp410_out: endpoint {
> > + remote-endpoint = <&hdmi_connector_in>;
> > + };
> > + };
> > + };
> > + };
> > + };
> > +
> > + hdmi: hdmi_connector {
> > + compatible = "hdmi-connector";
> > + label = "hdmi";
> > + type = "a";
> > + ddc-i2c-bus = <&i2c1>;
> > + port {
> > + hdmi_connector_in: endpoint {
> > + remote-endpoint = <&tfp410_out>;
> > + };
> > + };
> > + };
> > +
>
> You can drop the hdmi connector, the example will still validate.
Okay
>
> > ...
>
> --
> Regards,
>
> Laurent Pinchart
--
Jonathan Cormier
Software Engineer
Voice: 315.425.4045 x222
http://www.CriticalLink.com
6712 Brooklawn Parkway, Syracuse, NY 13211
On Wed, 25 Jan 2023 16:09:09 -0500, Jonathan Cormier wrote:
> Add a i2c example with HDMI connector
>
> Signed-off-by: Jonathan Cormier <[email protected]>
> ---
> .../bindings/display/bridge/ti,tfp410.yaml | 42 ++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/display/bridge/ti,tfp410.example.dts:77.37-79.19: ERROR (duplicate_label): /example-1/i2c/tfp410@38/ports/port@0/endpoint: Duplicate label 'tfp410_in' on /example-1/i2c/tfp410@38/ports/port@0/endpoint and /example-0/encoder/ports/port@0/endpoint
Documentation/devicetree/bindings/display/bridge/ti,tfp410.example.dts:84.38-86.19: ERROR (duplicate_label): /example-1/i2c/tfp410@38/ports/port@1/endpoint: Duplicate label 'tfp410_out' on /example-1/i2c/tfp410@38/ports/port@1/endpoint and /example-0/encoder/ports/port@1/endpoint
ERROR: Input tree has errors, aborting (use -f to force output)
make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/display/bridge/ti,tfp410.example.dtb] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1508: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On 25/01/2023 22:09, Jonathan Cormier wrote:
> Add a i2c example with HDMI connector
Why? It's the same - but more on this below.
>
> Signed-off-by: Jonathan Cormier <[email protected]>
You need to test the bindings before sending and fix the errors.
> ---
> .../bindings/display/bridge/ti,tfp410.yaml | 42 ++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> index 4c5dd8ec2951..456214f14b47 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> @@ -116,4 +116,46 @@ examples:
> };
> };
>
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + hdmi_encoder: tfp410@38 {
Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "ti,tfp410";
> + reg = <0x38>;
> +
> + ports {
> + address-cells = <1>;
> + size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + tfp410_in: endpoint {
> + remote-endpoint = <&dpi1_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + tfp410_out: endpoint {
> + remote-endpoint = <&hdmi_connector_in>;
> + };
That's the same example as existing one, so it looks useless. I don't
see benefits of this example.
> + };
> + };
> + };
> + };
> +
> + hdmi: hdmi_connector {
Drop. Incorrect name and not really related.
> + compatible = "hdmi-connector";
> + label = "hdmi";
> + type = "a";
> + ddc-i2c-bus = <&i2c1>;
> + port {
> + hdmi_connector_in: endpoint {
> + remote-endpoint = <&tfp410_out>;
> + };
> + };
> + };
> +
> ...
>
Best regards,
Krzysztof
On 25/01/2023 22:09, Jonathan Cormier wrote:
> From: Michael Williamson <[email protected]>
>
> The TFP410 driver does not support I2C. As such, the device remains in
> Power Down if the I2C is enabled by the bootstrap pins.
>
> Add basic support for the I2C interface, and provide support to take
> the device out of power down when enabled. Also read the bootstrap mode
> pins via the CTL_1_MODE register when using the I2C bus.
>
> Signed-off-by: Michael Williamson <[email protected]>
> Signed-off-by: Jonathan Cormier <[email protected]>
> ---
> drivers/gpu/drm/bridge/ti-tfp410.c | 95 +++++++++++++++++++++++++++-----------
> 1 file changed, 68 insertions(+), 27 deletions(-)
Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).
hint: it is entirely different.
>
> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> index b9635abbad16..323a6d9ed188 100644
> --- a/drivers/gpu/drm/bridge/ti-tfp410.c
> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> @@ -6,6 +6,7 @@
>
> #include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> +#include <linux/regmap.h>
> #include <linux/media-bus-format.h>
> #include <linux/module.h>
> #include <linux/of_graph.h>
> @@ -21,6 +22,20 @@
(...)
>
> static void tfp410_i2c_remove(struct i2c_client *client)
> @@ -408,7 +449,7 @@ MODULE_DEVICE_TABLE(i2c, tfp410_i2c_ids);
> static struct i2c_driver tfp410_i2c_driver = {
> .driver = {
> .name = "tfp410",
> - .of_match_table = of_match_ptr(tfp410_match),
> + .of_match_table = tfp410_match,
This does not look related to the patch.
Best regards,
Krzysztof
On 26/01/2023 19:36, Jon Cormier wrote:
> On Thu, Jan 26, 2023 at 10:40 AM Krzysztof Kozlowski <
> [email protected]> wrote:
>
>> On 25/01/2023 22:09, Jonathan Cormier wrote:
>>> Add a i2c example with HDMI connector
>>
>> Why? It's the same - but more on this below.
>>
> The existing example is for the previous setup where it was configured as
> its own device. It seemed necessary now that the driver is going to
> support being connected to an i2c bus to show it being used as such.
>
>>
>>>
>>> Signed-off-by: Jonathan Cormier <[email protected]>
>>
>> You need to test the bindings before sending and fix the errors.
>>
> Will do
>
>>
>>> ---
>>> .../bindings/display/bridge/ti,tfp410.yaml | 42
>> ++++++++++++++++++++++
>>> 1 file changed, 42 insertions(+)
>>>
>>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
>> b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
>>> index 4c5dd8ec2951..456214f14b47 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
>>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
>>> @@ -116,4 +116,46 @@ examples:
>>> };
>>> };
>>>
>>> + - |
>>> + i2c {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + hdmi_encoder: tfp410@38 {
>>
>> Node names should be generic.
>>
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>>
>> Can do
>
>>> + compatible = "ti,tfp410";
>>> + reg = <0x38>;
>>> +
>>> + ports {
>>> + address-cells = <1>;
>>> + size-cells = <0>;
>>> +
>>> + port@0 {
>>> + reg = <0>;
>>> + tfp410_in: endpoint {
>>> + remote-endpoint = <&dpi1_out>;
>>> + };
>>> + };
>>> +
>>> + port@1 {
>>> + reg = <1>;
>>> + tfp410_out: endpoint {
>>> + remote-endpoint = <&hdmi_connector_in>;
>>> + };
>>
>> That's the same example as existing one, so it looks useless. I don't
>> see benefits of this example.
>>
> It's mostly the same, except defined inside an i2c bus, with the reg value
> set. Without the powerdown-gpios or ti,deskew.
> And without the pclk-sample and bus-width (these are now read from i2c)
> And I included the hdmi_connector so it would be a more complete and useful
> example of how it could be used.
hdmi_connector is being dropped because it is not related.
> The TFP410 doesn't handle the ddc i2c bus
> on its own so a separate connector node is needed. I'll drop it if that's
> preferred.
>
If you had here different ports, it would be different case. But as of
now the only important part is having reg and not having gpios, so
basically almost the same example. No need for it.
Best regards,
Krzysztof
Hi Jonathan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on 93f875a8526a291005e7f38478079526c843cbec]
url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Cormier/dt-bindings-display-bridge-tfp410-Add-tfp410-i2c-example/20230128-183627
base: 93f875a8526a291005e7f38478079526c843cbec
patch link: https://lore.kernel.org/r/20230125-tfp410_i2c-v1-4-66a4d4e390b7%40criticallink.com
patch subject: [PATCH 4/4] DRM: BRIDGE: TFP410: If connected, use I2C for polled HPD status.
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230129/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/c4659fa4c02b62087c095ca99978e5eac8b490de
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jonathan-Cormier/dt-bindings-display-bridge-tfp410-Add-tfp410-i2c-example/20230128-183627
git checkout c4659fa4c02b62087c095ca99978e5eac8b490de
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 olddefconfig
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers/gpu/drm/bridge/ti-tfp410.c: In function 'tfp410_connector_detect':
>> drivers/gpu/drm/bridge/ti-tfp410.c:111:13: warning: unused variable 'val' [-Wunused-variable]
111 | u32 val;
| ^~~
vim +/val +111 drivers/gpu/drm/bridge/ti-tfp410.c
106
107 static enum drm_connector_status
108 tfp410_connector_detect(struct drm_connector *connector, bool force)
109 {
110 struct tfp410 *dvi = drm_connector_to_tfp410(connector);
> 111 u32 val;
112 unsigned int ret;
113
114 if (dvi->i2c) {
115 ret = regmap_test_bits(dvi->regmap, TFP410_REG_CTL_2_MODE, TFP410_BIT_HTPLG);
116 if (ret < 0)
117 dev_err(dvi->dev, "%s failed to read HTPLG bit : %d\n", __func__, ret);
118 else
119 return ret ? connector_status_connected : connector_status_disconnected;
120 }
121
122 return drm_bridge_detect(dvi->next_bridge);
123 }
124
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi Jonathan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on 93f875a8526a291005e7f38478079526c843cbec]
url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Cormier/dt-bindings-display-bridge-tfp410-Add-tfp410-i2c-example/20230128-183627
base: 93f875a8526a291005e7f38478079526c843cbec
patch link: https://lore.kernel.org/r/20230125-tfp410_i2c-v1-4-66a4d4e390b7%40criticallink.com
patch subject: [PATCH 4/4] DRM: BRIDGE: TFP410: If connected, use I2C for polled HPD status.
config: i386-randconfig-a006 (https://download.01.org/0day-ci/archive/20230129/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c4659fa4c02b62087c095ca99978e5eac8b490de
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jonathan-Cormier/dt-bindings-display-bridge-tfp410-Add-tfp410-i2c-example/20230128-183627
git checkout c4659fa4c02b62087c095ca99978e5eac8b490de
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/bridge/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/bridge/ti-tfp410.c:111:6: warning: unused variable 'val' [-Wunused-variable]
u32 val;
^
1 warning generated.
vim +/val +111 drivers/gpu/drm/bridge/ti-tfp410.c
106
107 static enum drm_connector_status
108 tfp410_connector_detect(struct drm_connector *connector, bool force)
109 {
110 struct tfp410 *dvi = drm_connector_to_tfp410(connector);
> 111 u32 val;
112 unsigned int ret;
113
114 if (dvi->i2c) {
115 ret = regmap_test_bits(dvi->regmap, TFP410_REG_CTL_2_MODE, TFP410_BIT_HTPLG);
116 if (ret < 0)
117 dev_err(dvi->dev, "%s failed to read HTPLG bit : %d\n", __func__, ret);
118 else
119 return ret ? connector_status_connected : connector_status_disconnected;
120 }
121
122 return drm_bridge_detect(dvi->next_bridge);
123 }
124
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On Sat, Jan 28, 2023 at 7:47 PM kernel test robot <[email protected]> wrote:
>
> Hi Jonathan,
>
> Thank you for the patch! Perhaps something to improve:
Good bot.
>
> [auto build test WARNING on 93f875a8526a291005e7f38478079526c843cbec]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Cormier/dt-bindings-display-bridge-tfp410-Add-tfp410-i2c-example/20230128-183627
> base: 93f875a8526a291005e7f38478079526c843cbec
> patch link: https://lore.kernel.org/r/20230125-tfp410_i2c-v1-4-66a4d4e390b7%40criticallink.com
> patch subject: [PATCH 4/4] DRM: BRIDGE: TFP410: If connected, use I2C for polled HPD status.
> config: i386-randconfig-a006 (https://download.01.org/0day-ci/archive/20230129/[email protected]/config)
> compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/c4659fa4c02b62087c095ca99978e5eac8b490de
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Jonathan-Cormier/dt-bindings-display-bridge-tfp410-Add-tfp410-i2c-example/20230128-183627
> git checkout c4659fa4c02b62087c095ca99978e5eac8b490de
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/bridge/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/gpu/drm/bridge/ti-tfp410.c:111:6: warning: unused variable 'val' [-Wunused-variable]
> u32 val;
> ^
> 1 warning generated.
This has already been fixed in V2 of the patch series.
>
>
> vim +/val +111 drivers/gpu/drm/bridge/ti-tfp410.c
>
> 106
> 107 static enum drm_connector_status
> 108 tfp410_connector_detect(struct drm_connector *connector, bool force)
> 109 {
> 110 struct tfp410 *dvi = drm_connector_to_tfp410(connector);
> > 111 u32 val;
> 112 unsigned int ret;
> 113
> 114 if (dvi->i2c) {
> 115 ret = regmap_test_bits(dvi->regmap, TFP410_REG_CTL_2_MODE, TFP410_BIT_HTPLG);
> 116 if (ret < 0)
> 117 dev_err(dvi->dev, "%s failed to read HTPLG bit : %d\n", __func__, ret);
> 118 else
> 119 return ret ? connector_status_connected : connector_status_disconnected;
> 120 }
> 121
> 122 return drm_bridge_detect(dvi->next_bridge);
> 123 }
> 124
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests