* [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints
@ 2023-03-31 9:11 Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 01/10] device property: Add remote endpoint to devcon matcher Pin-yen Lin
` (9 more replies)
0 siblings, 10 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel, Alex Deucher, Allen Chen, David Zhang,
Dmitry Baryshkov, Douglas Anderson, Imre Deak, Jani Nikula,
Linus Walleij, Uwe Kleine-König, YueHaibing
This series introduces bindings for anx7625/it6505 to register Type-C
mode-switch in their output endpoints, and use data-lanes property to
describe the pin connections.
This series is not directly related to the built-in mux in anx7625,
which automatically switches between the two orientations of a single
Type-C connector. This series adds support of registering mode switches
for two downstream devices, while we use orientation switches for two
orientations of the Type-C connector.
The first two patch modifies fwnode_graph_devcon_matches and
cros_typec_init_ports to enable the registration of the switches.
Patch 4~6 introduce the bindings for anx7625 and the corresponding driver
modifications.
Patch 7~9 add similar bindings and driver changes for it6505.
v14: https://lore.kernel.org/all/20230322104639.221402-1-treapking@chromium.org/
v13: https://lore.kernel.org/all/20230303143350.815623-1-treapking@chromium.org/
v12: https://lore.kernel.org/all/20230221095054.1868277-1-treapking@chromium.org/
v11: https://lore.kernel.org/all/20230204133040.1236799-1-treapking@chromium.org/
v10: https://lore.kernel.org/all/20230112042104.4107253-1-treapking@chromium.org/
v9: https://lore.kernel.org/all/20230109084101.265664-1-treapking@chromium.org/
v8: https://lore.kernel.org/all/20230107102231.23682-1-treapking@chromium.org/
v7: https://lore.kernel.org/all/20230105132457.4125372-1-treapking@chromium.org/
v6: https://lore.kernel.org/all/20221124102056.393220-1-treapking@chromium.org/
v5: https://lore.kernel.org/linux-usb/20220622173605.1168416-1-pmalani@chromium.org/
Changes in v15:
- Collected Reviewed-by tags
- Change the struct definition order for the Type-C switch helpers
- Fix style issues
- Updated the docs for the return value
- Updated the description in Kconfig
- Use dev_name as the typec_mux name
- Swap the definitions in anx7625_typec_port_data
- Add comments about the completion
- Abort the mux_set callback when the switch is unregistered
Changes in v14:
- Collect review tags
- Introduce a new Kconfig becuase it didn't build when CONFIG_TYPEC=m
- Add comments about devm_* usage
- Fix style issues
Changes in v13:
- Update the kernel doc of fwnode_connection_find_match
- Add typec_mode_switch_node_count helper
- Fix style issues
- Update a typo in the commit message
- Collect Reviewed-by tag
Changes in v12:
- Check the availability of the device node in fwnode_graph_devcon_matches
- Ensured valid access to "matches" in fwnode_graph_devcon_matches
- Updated the documentation in fwnode_connection_find_match(es)
- Add fwnode_for_each_typec_mode_switch macro
- Remove a duplicated dmesg in the helper
- Used IS_REACHABLE instead to guard the function signatures
- Removed the 4-lane binding in analogix,anx7625.yaml
- Reworded the description for the mode-switch property
- Fixed style issues in anx7625 driver
- Fixed the inverted orientation setting in anx7625 driver
- Changed "&ctx->client->dev" to "ctx->dev"
- Fixed the schema of "data-lanes" property for it6505
- Fixes style issues in it6505 driver
- Replaced &it6505->client->dev with it6505->dev
- Updated the error logs when parsing data-lanes property
Changes in v11:
- Added missing fwnode_handle_put in drivers/base/property.c
- Collected Acked-by tag
- Use fwnode helpers instead of DT
- Moved the helpers to a new file
- Use "reg" instead of "data-lanes" to determine the port number
- Updated the description of the endpoints in the bindings
- Referenced video-interfaces.yaml instead for the endpoints binding
- Removed duplicated definitions from inherited schema
- Moved the "data-lanes" parsing logics to bridge drivers
- Removed Kconfig dependencies for the bridge drivers
- Updated the usage of the private bridge driver data
- Added a clarification on the anx7625 built-in mux in the cover letter
Changes in v10:
- Collected Reviewed-by and Tested-by tags
- Replaced "void *" with "typec_mux_set_fn_t" for mux_set callbacks
- Print out the node name when errors on parsing DT
- Use dev_dbg instead of dev_warn when no Type-C switch nodes available
- Made the return path of drm_dp_register_mode_switch clearer
- Added a TODO for implementing orientation switch for anx7625
- Updated the commit message for the absence of orientation switch
- Fixed typo in the commit message
Changes in v9:
- Collected Reviewed-by tag
- Fixed subject prefix again
- Changed the naming of the example node for it6505
Changes in v8:
- Fixed the build issue when CONFIG_TYPEC=m
- Fixed some style issues
- Fixed the subject prefixes for the bindings patch
- Fixed the bindings for data-lanes properties
Changes in v7:
- Fix the long comment lines
- Extracted the common codes to a helper function
- Fixed style issues in anx7625 driver
- Removed DT property validation in anx7625 driver.
- Fixed style issues in it6505 driver
- Removed the redundant sleep in it6505 driver
- Removed DT property validation in it6505 driver
- Rebased to drm-misc-next
- Fixed indentations in bindings patches
- Added a new patch to fix indentations in Kconfig
Changes in v6:
- Changed it6505_typec_mux_set callback function to accommodate with
the latest drm-misc patches
- Changed the driver implementation to accommodate with the new binding
- Dropped typec-switch binding and use endpoints and data-lanes properties
to describe the pin connections
- Added new patches (patch 1,2,4) to fix probing issues
- Changed the bindings of it6505/anx7625 and modified the drivers
accordingly
- Merged it6505/anx7625 driver changes into a single patch
Pin-yen Lin (8):
drm/display: Add Type-C switch helpers
dt-bindings: display: bridge: anx7625: Add mode-switch support
drm/bridge: anx7625: Check for Type-C during panel registration
drm/bridge: Remove redundant i2c_client in anx7625/it6505
drm/bridge: anx7625: Register Type C mode switches
dt-bindings: display: bridge: it6505: Add mode-switch support
drm/bridge: it6505: Fix Kconfig indentation
drm/bridge: it6505: Register Type C mode switches
Prashant Malani (2):
device property: Add remote endpoint to devcon matcher
platform/chrome: cros_ec_typec: Purge blocking switch devlinks
.../display/bridge/analogix,anx7625.yaml | 88 ++++-
.../bindings/display/bridge/ite,it6505.yaml | 101 +++++-
drivers/base/property.c | 31 +-
drivers/gpu/drm/bridge/Kconfig | 20 +-
drivers/gpu/drm/bridge/analogix/anx7625.c | 265 +++++++++++---
drivers/gpu/drm/bridge/analogix/anx7625.h | 22 +-
drivers/gpu/drm/bridge/ite-it6505.c | 326 ++++++++++++++----
drivers/gpu/drm/display/Kconfig | 8 +
drivers/gpu/drm/display/Makefile | 2 +
drivers/gpu/drm/display/drm_dp_typec_helper.c | 107 ++++++
drivers/platform/chrome/cros_ec_typec.c | 10 +
include/drm/display/drm_dp_helper.h | 46 +++
12 files changed, 874 insertions(+), 152 deletions(-)
create mode 100644 drivers/gpu/drm/display/drm_dp_typec_helper.c
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v15 01/10] device property: Add remote endpoint to devcon matcher
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
@ 2023-03-31 9:11 ` Pin-yen Lin
2023-04-07 21:54 ` Stephen Boyd
2023-03-31 9:11 ` [PATCH v15 02/10] platform/chrome: cros_ec_typec: Purge blocking switch devlinks Pin-yen Lin
` (8 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel
From: Prashant Malani <pmalani@chromium.org>
When searching the device graph for device matches, check the
remote-endpoint itself for a match.
Some drivers register devices for individual endpoints. This allows
the matcher code to evaluate those for a match too, instead
of only looking at the remote parent devices. This is required when a
device supports two mode switches in its endpoints, so we can't simply
register the mode switch with the parent node.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
(no changes since v14)
Changes in v14:
- Collect review tags
Changes in v13:
- Update the kernel doc of fwnode_connection_find_match
Changes in v12:
- Check the availability of the device node in fwnode_graph_devcon_matches
- Ensured valid access to "matches" in fwnode_graph_devcon_matches
- Updated the documentation in fwnode_connection_find_match(es)
- Dropped collected tags due to the new changes
Changes in v11:
- Added missing fwnode_handle_put in drivers/base/property.c
Changes in v10:
- Collected Reviewed-by and Tested-by tags
Changes in v6:
- New in v6
drivers/base/property.c | 31 ++++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 083a95791d3b..4426ac2b16ca 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1243,6 +1243,23 @@ static unsigned int fwnode_graph_devcon_matches(const struct fwnode_handle *fwno
continue;
}
+ ret = match(node, con_id, data);
+ fwnode_handle_put(node);
+ if (ret) {
+ if (matches)
+ matches[count] = ret;
+ count++;
+
+ if (matches && count >= matches_len)
+ break;
+ }
+
+ /*
+ * Some drivers may register devices for endpoints. Check
+ * the remote-endpoints for matches in addition to the remote
+ * port parent.
+ */
+ node = fwnode_graph_get_remote_endpoint(ep);
ret = match(node, con_id, data);
fwnode_handle_put(node);
if (ret) {
@@ -1293,8 +1310,11 @@ static unsigned int fwnode_devcon_matches(const struct fwnode_handle *fwnode,
* @match: Function to check and convert the connection description
*
* Find a connection with unique identifier @con_id between @fwnode and another
- * device node. @match will be used to convert the connection description to
- * data the caller is expecting to be returned.
+ * device node. For fwnode graph connections, the graph endpoints are also
+ * checked. @match will be used to convert the connection description to data
+ * the caller is expecting to be returned.
+ *
+ * Return: The pointer to the matched node, or NULL on error.
*/
void *fwnode_connection_find_match(const struct fwnode_handle *fwnode,
const char *con_id, void *data,
@@ -1325,9 +1345,10 @@ EXPORT_SYMBOL_GPL(fwnode_connection_find_match);
* @matches_len: Length of @matches
*
* Find up to @matches_len connections with unique identifier @con_id between
- * @fwnode and other device nodes. @match will be used to convert the
- * connection description to data the caller is expecting to be returned
- * through the @matches array.
+ * @fwnode and other device nodes. For fwnode graph connections, the graph
+ * endpoints are also checked. @match will be used to convert the connection
+ * description to data the caller is expecting to be returned through the
+ * @matches array.
* If @matches is NULL @matches_len is ignored and the total number of resolved
* matches is returned.
*
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v15 02/10] platform/chrome: cros_ec_typec: Purge blocking switch devlinks
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 01/10] device property: Add remote endpoint to devcon matcher Pin-yen Lin
@ 2023-03-31 9:11 ` Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 03/10] drm/display: Add Type-C switch helpers Pin-yen Lin
` (7 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel, Chen-Yu Tsai
From: Prashant Malani <pmalani@chromium.org>
When using OF graph, the fw_devlink code will create links between the
individual port driver (cros-ec-typec here) and the parent device for
a Type-C switch (like mode-switch). Since the mode-switch will in turn
have the usb-c-connector (i.e the child of the port driver) as a
supplier, fw_devlink will not be able to resolve the cyclic dependency
correctly.
As a result, the mode-switch driver probe() never runs, so mode-switches
are never registered. Because of that, the port driver probe constantly
fails with -EPROBE_DEFER, because the Type-C connector class requires all
switch devices to be registered prior to port registration.
To break this deadlock and allow the mode-switch registration to occur,
purge all the usb-c-connector nodes' absent suppliers. This eliminates
the connector as a supplier for a switch and allows it to be probed.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
Changes in v15:
- Collected Reviewed-by tags
Changes in v11:
- Collected Acked-by tag
Changes in v10:
- Collected Reviewed-by and Tested-by tags
Changes in v7:
- Fix the long comment lines
Changes in v6:
- New in v6
drivers/platform/chrome/cros_ec_typec.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index a673c3342470..5911cd9640cb 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -325,6 +325,16 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
return -EINVAL;
}
+ /*
+ * OF graph may have set up some device links with switches,
+ * since connectors have their own compatible. Purge these
+ * to avoid a deadlock in switch probe (the switch mistakenly
+ * assumes the connector is a supplier).
+ */
+ if (dev_of_node(dev))
+ device_for_each_child_node(dev, fwnode)
+ fw_devlink_purge_absent_suppliers(fwnode);
+
/* DT uses "reg" to specify port number. */
port_prop = dev->of_node ? "reg" : "port-number";
device_for_each_child_node(dev, fwnode) {
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v15 03/10] drm/display: Add Type-C switch helpers
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 01/10] device property: Add remote endpoint to devcon matcher Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 02/10] platform/chrome: cros_ec_typec: Purge blocking switch devlinks Pin-yen Lin
@ 2023-03-31 9:11 ` Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support Pin-yen Lin
` (6 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel, Alex Deucher, David Zhang,
Dmitry Baryshkov, Douglas Anderson, Imre Deak, Jani Nikula,
Linus Walleij, YueHaibing
Add helpers to register and unregister Type-C "switches" for bridges
capable of switching their output between two downstream devices.
The helper registers USB Type-C mode switches when the "mode-switch"
and the "reg" properties are available in Device Tree.
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
---
Changes in v15:
- Change the struct definition order for the Type-C switch helpers
- Fix style issues
- Updated the docs for the return value
- Updated the description in Kconfig
- Use dev_name as the typec_mux name
Changes in v14:
- Introduce a new Kconfig because it didn't build when CONFIG_TYPEC=m
- Add comments about devm_* usage
- Fix style issues
Changes in v13:
- Add typec_mode_switch_node_count helper
- Fix style issues
Changes in v12:
- Add fwnode_for_each_typec_mode_switch macro
- Remove a duplicated dmesg in the helper
- Used IS_REACHABLE instead to guard the function signatures
Changes in v11:
- Use fwnode helpers instead of DT
- Moved the helpers to a new file
- Use "reg" instead of "data-lanes" to determine the port number
- Dropped collected tags due to new changes
Changes in v10:
- Collected Reviewed-by and Tested-by tags
- Replaced "void *" with "typec_mux_set_fn_t" for mux_set callbacks
- Print out the node name when errors on parsing DT
- Use dev_dbg instead of dev_warn when no Type-C switch nodes available
- Made the return path of drm_dp_register_mode_switch clearer
Changes in v8:
- Fixed the build issue when CONFIG_TYPEC=m
- Fixed some style issues
Changes in v7:
- Extracted the common codes to a helper function
- New in v7
drivers/gpu/drm/display/Kconfig | 8 ++
drivers/gpu/drm/display/Makefile | 2 +
drivers/gpu/drm/display/drm_dp_typec_helper.c | 107 ++++++++++++++++++
include/drm/display/drm_dp_helper.h | 46 ++++++++
4 files changed, 163 insertions(+)
create mode 100644 drivers/gpu/drm/display/drm_dp_typec_helper.c
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 09712b88a5b8..4eaf00380a1f 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -29,6 +29,14 @@ config DRM_DISPLAY_HDMI_HELPER
help
DRM display helpers for HDMI.
+config DRM_DISPLAY_DP_TYPEC_HELPER
+ def_bool y
+ depends on DRM_DISPLAY_HELPER
+ depends on DRM_DISPLAY_HELPER=TYPEC || TYPEC=y
+ help
+ Choose this option to enable helpers for registering USB Type-C
+ switches when USB Type-C DisplayPort Alternate mode is used.
+
config DRM_DP_AUX_CHARDEV
bool "DRM DP AUX Interface"
depends on DRM && DRM_DISPLAY_HELPER
diff --git a/drivers/gpu/drm/display/Makefile b/drivers/gpu/drm/display/Makefile
index 17ac4a1006a8..2202a6aea38e 100644
--- a/drivers/gpu/drm/display/Makefile
+++ b/drivers/gpu/drm/display/Makefile
@@ -8,6 +8,8 @@ drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_HELPER) += \
drm_dp_helper.o \
drm_dp_mst_topology.o \
drm_dsc_helper.o
+drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_TYPEC_HELPER) += \
+ drm_dp_typec_helper.o
drm_display_helper-$(CONFIG_DRM_DISPLAY_HDCP_HELPER) += drm_hdcp_helper.o
drm_display_helper-$(CONFIG_DRM_DISPLAY_HDMI_HELPER) += \
drm_hdmi_helper.o \
diff --git a/drivers/gpu/drm/display/drm_dp_typec_helper.c b/drivers/gpu/drm/display/drm_dp_typec_helper.c
new file mode 100644
index 000000000000..48c1bbf57b2d
--- /dev/null
+++ b/drivers/gpu/drm/display/drm_dp_typec_helper.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/usb/typec_mux.h>
+#include <drm/display/drm_dp_helper.h>
+
+static int drm_dp_register_mode_switch(struct device *dev,
+ struct fwnode_handle *fwnode,
+ struct drm_dp_typec_switch_desc *switch_desc,
+ void *data, typec_mux_set_fn_t mux_set)
+{
+ struct drm_dp_typec_port_data *port_data;
+ struct typec_mux_desc mux_desc = {};
+ char name[32];
+ u32 port_num;
+ int ret;
+
+ ret = fwnode_property_read_u32(fwnode, "reg", &port_num);
+ if (ret) {
+ dev_err(dev, "Failed to read reg property: %d\n", ret);
+ return ret;
+ }
+
+ port_data = &switch_desc->typec_ports[port_num];
+ port_data->data = data;
+ port_data->port_num = port_num;
+ port_data->fwnode = fwnode;
+ mux_desc.fwnode = fwnode;
+ mux_desc.drvdata = port_data;
+ snprintf(name, sizeof(name), "%s-%u", dev_name(dev), port_num);
+ mux_desc.name = name;
+ mux_desc.set = mux_set;
+
+ port_data->typec_mux = typec_mux_register(dev, &mux_desc);
+ ret = PTR_ERR_OR_ZERO(port_data->typec_mux);
+ if (ret)
+ dev_err(dev, "Mode switch register for port %d failed: %d\n",
+ port_num, ret);
+
+ return ret;
+}
+
+/**
+ * drm_dp_register_typec_switches() - register Type-C switches
+ * @dev: Device that registers Type-C switches
+ * @port: Device node for the switch
+ * @switch_desc: A Type-C switch descriptor
+ * @data: Private data for the switches
+ * @mux_set: Callback function for typec_mux_set
+ *
+ * This function registers USB Type-C switches for DP bridges that can switch
+ * the output signal between their output pins. This function uses devm_kcalloc
+ * to allocate memory, so it is expected to only call this in the driver probe
+ * functions.
+ *
+ * Currently only mode switches are implemented, and the function assumes the
+ * given @port device node has endpoints with "mode-switch" property.
+ * The port number is determined by the "reg" property of the endpoint.
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_dp_register_typec_switches(struct device *dev, struct fwnode_handle *port,
+ struct drm_dp_typec_switch_desc *switch_desc,
+ void *data, typec_mux_set_fn_t mux_set)
+{
+ struct fwnode_handle *sw;
+ int ret;
+
+ switch_desc->num_typec_switches = typec_mode_switch_node_count(port);
+ if (!switch_desc->num_typec_switches) {
+ dev_dbg(dev, "No Type-C switches node found\n");
+ return 0;
+ }
+
+ switch_desc->typec_ports = devm_kcalloc(dev, switch_desc->num_typec_switches,
+ sizeof(*switch_desc->typec_ports),
+ GFP_KERNEL);
+ if (!switch_desc->typec_ports)
+ return -ENOMEM;
+
+ /* Register switches for each connector. */
+ for_each_typec_mode_switch_node(port, sw) {
+ ret = drm_dp_register_mode_switch(dev, sw, switch_desc, data, mux_set);
+ if (ret)
+ goto err_unregister_typec_switches;
+ }
+
+ return 0;
+
+err_unregister_typec_switches:
+ fwnode_handle_put(sw);
+ drm_dp_unregister_typec_switches(switch_desc);
+ return ret;
+}
+EXPORT_SYMBOL(drm_dp_register_typec_switches);
+
+/**
+ * drm_dp_unregister_typec_switches() - unregister Type-C switches
+ * @switch_desc: A Type-C switch descriptor
+ */
+void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
+{
+ unsigned int i;
+
+ for (i = 0; i < switch_desc->num_typec_switches; i++)
+ typec_mux_unregister(switch_desc->typec_ports[i].typec_mux);
+}
+EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index ab55453f2d2c..8c2d50c05335 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -25,6 +25,7 @@
#include <linux/delay.h>
#include <linux/i2c.h>
+#include <linux/usb/typec_mux.h>
#include <drm/display/drm_dp.h>
#include <drm/drm_connector.h>
@@ -763,4 +764,49 @@ bool drm_dp_downstream_rgb_to_ycbcr_conversion(const u8 dpcd[DP_RECEIVER_CAP_SIZ
const u8 port_cap[4], u8 color_spc);
int drm_dp_pcon_convert_rgb_to_ycbcr(struct drm_dp_aux *aux, u8 color_spc);
+struct drm_dp_typec_port_data {
+ struct typec_mux_dev *typec_mux;
+ struct fwnode_handle *fwnode;
+ void *data;
+ int port_num;
+};
+
+struct drm_dp_typec_switch_desc {
+ struct drm_dp_typec_port_data *typec_ports;
+ int num_typec_switches;
+};
+
+#define for_each_typec_mode_switch_node(port, sw) \
+ fwnode_for_each_child_node((port), (sw)) \
+ for_each_if(fwnode_property_present((sw), "mode-switch"))
+
+static inline unsigned int typec_mode_switch_node_count(struct fwnode_handle *port)
+{
+ struct fwnode_handle *sw;
+ unsigned int count = 0;
+
+ for_each_typec_mode_switch_node(port, sw)
+ count++;
+
+ return count;
+}
+
+#ifdef CONFIG_DRM_DISPLAY_DP_TYPEC_HELPER
+void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
+int drm_dp_register_typec_switches(struct device *dev, struct fwnode_handle *port,
+ struct drm_dp_typec_switch_desc *switch_desc,
+ void *data, typec_mux_set_fn_t mux_set);
+#else
+static inline void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
+{
+}
+static inline int drm_dp_register_typec_switches(
+ struct device *dev, struct fwnode_handle *port,
+ struct drm_dp_typec_switch_desc *switch_desc, void *data,
+ typec_mux_set_fn_t mux_set)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
#endif /* _DRM_DP_HELPER_H_ */
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
` (2 preceding siblings ...)
2023-03-31 9:11 ` [PATCH v15 03/10] drm/display: Add Type-C switch helpers Pin-yen Lin
@ 2023-03-31 9:11 ` Pin-yen Lin
2023-04-06 15:18 ` Rob Herring
2023-04-12 1:38 ` Stephen Boyd
2023-03-31 9:11 ` [PATCH v15 05/10] drm/bridge: anx7625: Check for Type-C during panel registration Pin-yen Lin
` (5 subsequent siblings)
9 siblings, 2 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai
Analogix 7625 can be used in systems to switch the DP traffic between
two downstreams, which can be USB Type-C DisplayPort alternate mode
lane or regular DisplayPort output ports.
Update the binding to accommodate this usage by introducing a
data-lanes and a mode-switch property on endpoints.
Also include the link to the product brief in the bindings.
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
(no changes since v12)
Changes in v12:
- Removed the 4-lane binding in analogix,anx7625.yaml
- Reworded the description for the mode-switch property
Changes in v11:
- Updated the description of the endpoints
- Referenced video-interfaces.yaml instead for the endpoints
Changes in v10:
- Collected Reviewed-by and Tested-by tags
Changes in v9:
- Collected Reviewed-by tag
Changes in v8:
- Updated anx7625 bindings for data-lane property
- Fixed the subject prefix
Changes in v7:
- Fixed issues reported by dt_binding_check
- Updated the schema and the example dts for data-lanes.
- Changed to generic naming for the example dts node.
Changes in v6:
- Remove switches node and use endpoints and data-lanes property to
describe the connections.
.../display/bridge/analogix,anx7625.yaml | 88 ++++++++++++++++++-
1 file changed, 85 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
index b42553ac505c..604c7391d74f 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
@@ -12,7 +12,8 @@ maintainers:
description: |
The ANX7625 is an ultra-low power 4K Mobile HD Transmitter
- designed for portable devices.
+ designed for portable devices. Product brief is available at
+ https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf
properties:
compatible:
@@ -112,9 +113,40 @@ properties:
data-lanes: true
port@1:
- $ref: /schemas/graph.yaml#/properties/port
+ $ref: /schemas/graph.yaml#/$defs/port-base
description:
- Video port for panel or connector.
+ Video port for panel or connector. Each endpoint connects to a video
+ output downstream, and the "data-lanes" property is used to describe
+ the pin connections. 0, 1, 2, 3 in "data-lanes" maps to SSRX1, SSTX1,
+ SSRX2, SSTX2, respectively.
+
+ patternProperties:
+ "^endpoint@[01]$":
+ $ref: /schemas/media/video-interfaces.yaml#
+ properties:
+ reg: true
+
+ remote-endpoint: true
+
+ data-lanes:
+ oneOf:
+ - items:
+ - enum: [0, 1, 2, 3]
+
+ - items:
+ - const: 0
+ - const: 1
+
+ - items:
+ - const: 2
+ - const: 3
+
+ mode-switch:
+ type: boolean
+ description: Serves as Type-C mode switch if present.
+
+ required:
+ - remote-endpoint
required:
- port@0
@@ -186,3 +218,53 @@ examples:
};
};
};
+ - |
+ i2c3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ encoder@58 {
+ compatible = "analogix,anx7625";
+ reg = <0x58>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&anx7625_dp_pins>;
+ enable-gpios = <&pio 176 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&pio 177 GPIO_ACTIVE_HIGH>;
+ vdd10-supply = <&pp1100_dpbrdg>;
+ vdd18-supply = <&pp1800_dpbrdg_dx>;
+ vdd33-supply = <&pp3300_dpbrdg_dx>;
+ analogix,audio-enable;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ anx7625_dp_in: endpoint {
+ bus-type = <7>;
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+
+ port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg = <1>;
+ anx_typec0: endpoint@0 {
+ reg = <0>;
+ mode-switch;
+ data-lanes = <0 1>;
+ remote-endpoint = <&typec_port0>;
+ };
+ anx_typec1: endpoint@1 {
+ reg = <1>;
+ mode-switch;
+ data-lanes = <2 3>;
+ remote-endpoint = <&typec_port1>;
+ };
+ };
+ };
+ };
+ };
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v15 05/10] drm/bridge: anx7625: Check for Type-C during panel registration
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
` (3 preceding siblings ...)
2023-03-31 9:11 ` [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support Pin-yen Lin
@ 2023-03-31 9:11 ` Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 06/10] drm/bridge: Remove redundant i2c_client in anx7625/it6505 Pin-yen Lin
` (4 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel, Uwe Kleine-König
The output port endpoints can be connected to USB-C connectors.
Running drm_of_find_panel_or_bridge() with such endpoints leads to
a continuous return value of -EPROBE_DEFER, even though there is
no panel present.
To avoid this, check for the existence of a "mode-switch" property in
the port endpoint, and skip panel registration completely if so.
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
Changes in v15:
- Collect review tag
Changes in v14:
- Collect review tag
Changes in v13:
- Use the new typec_mode_switch_node_count helper
Changes in v12:
- Updated to use fwnode_for_each_typec_mode_switch macro
- Dropped collected tags
Changes in v10:
- Collected Reviewed-by and Tested-by tags
Changes in v6:
- New in v6
drivers/gpu/drm/bridge/analogix/anx7625.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 6846199a2ee1..3f6bf7674d32 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1648,7 +1648,8 @@ static int anx7625_get_swing_setting(struct device *dev,
static int anx7625_parse_dt(struct device *dev,
struct anx7625_platform_data *pdata)
{
- struct device_node *np = dev->of_node, *ep0;
+ struct device_node *np = dev->of_node, *ep0, *port_node;
+ unsigned int count;
int bus_type, mipi_lanes;
anx7625_get_swing_setting(dev, pdata);
@@ -1687,6 +1688,15 @@ static int anx7625_parse_dt(struct device *dev,
if (of_property_read_bool(np, "analogix,audio-enable"))
pdata->audio_en = 1;
+ /*
+ * Don't bother finding a panel if a Type-C `mode-switch` property is
+ * present in one of the endpoints in the output port.
+ */
+ port_node = of_graph_get_port_by_id(np, 1);
+ count = typec_mode_switch_node_count(&port_node->fwnode);
+ if (count)
+ return 0;
+
pdata->panel_bridge = devm_drm_of_get_bridge(dev, np, 1, 0);
if (IS_ERR(pdata->panel_bridge)) {
if (PTR_ERR(pdata->panel_bridge) == -ENODEV) {
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v15 06/10] drm/bridge: Remove redundant i2c_client in anx7625/it6505
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
` (4 preceding siblings ...)
2023-03-31 9:11 ` [PATCH v15 05/10] drm/bridge: anx7625: Check for Type-C during panel registration Pin-yen Lin
@ 2023-03-31 9:11 ` Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 07/10] drm/bridge: anx7625: Register Type C mode switches Pin-yen Lin
` (3 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel
These two drivers embed a i2c_client in their private driver data, but
only strict device is actually needed. Replace the i2c_client reference
with a struct device one.
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
Changes in v15:
- Collect review tag
Changes in v13:
- Update a typo in the commit message
- Collect Reviewed-by tag
Changes in v12:
- New in v12
drivers/gpu/drm/bridge/analogix/anx7625.c | 96 ++++++++--------
drivers/gpu/drm/bridge/analogix/anx7625.h | 2 +-
drivers/gpu/drm/bridge/ite-it6505.c | 128 +++++++++++-----------
3 files changed, 113 insertions(+), 113 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 3f6bf7674d32..76d46db3f8dc 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -207,7 +207,7 @@ static int anx7625_read_ctrl_status_p0(struct anx7625_data *ctx)
static int wait_aux_op_finish(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
int val;
int ret;
@@ -234,7 +234,7 @@ static int wait_aux_op_finish(struct anx7625_data *ctx)
static int anx7625_aux_trans(struct anx7625_data *ctx, u8 op, u32 address,
u8 len, u8 *buf)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
int ret;
u8 addrh, addrm, addrl;
u8 cmd;
@@ -427,7 +427,7 @@ static int anx7625_odfc_config(struct anx7625_data *ctx,
u8 post_divider)
{
int ret;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
/* Config input reference clock frequency 27MHz/19.2MHz */
ret = anx7625_write_and(ctx, ctx->i2c.rx_p1_client, MIPI_DIGITAL_PLL_16,
@@ -477,7 +477,7 @@ static int anx7625_set_k_value(struct anx7625_data *ctx)
static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
unsigned long m, n;
u16 htotal;
int ret;
@@ -575,7 +575,7 @@ static int anx7625_dsi_video_timing_config(struct anx7625_data *ctx)
static int anx7625_swap_dsi_lane3(struct anx7625_data *ctx)
{
int val;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
/* Swap MIPI-DSI data lane 3 P and N */
val = anx7625_reg_read(ctx, ctx->i2c.rx_p1_client, MIPI_SWAP);
@@ -592,7 +592,7 @@ static int anx7625_api_dsi_config(struct anx7625_data *ctx)
{
int val, ret;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
/* Swap MIPI-DSI data lane 3 P and N */
ret = anx7625_swap_dsi_lane3(ctx);
@@ -657,7 +657,7 @@ static int anx7625_api_dsi_config(struct anx7625_data *ctx)
static int anx7625_dsi_config(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
int ret;
DRM_DEV_DEBUG_DRIVER(dev, "config dsi.\n");
@@ -689,7 +689,7 @@ static int anx7625_dsi_config(struct anx7625_data *ctx)
static int anx7625_api_dpi_config(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
u16 freq = ctx->dt.pixelclock.min / 1000;
int ret;
@@ -720,7 +720,7 @@ static int anx7625_api_dpi_config(struct anx7625_data *ctx)
static int anx7625_dpi_config(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
int ret;
DRM_DEV_DEBUG_DRIVER(dev, "config dpi\n");
@@ -765,7 +765,7 @@ static int anx7625_read_flash_status(struct anx7625_data *ctx)
static int anx7625_hdcp_key_probe(struct anx7625_data *ctx)
{
int ret, val;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
u8 ident[FLASH_BUF_LEN];
ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
@@ -815,7 +815,7 @@ static int anx7625_hdcp_key_probe(struct anx7625_data *ctx)
static int anx7625_hdcp_key_load(struct anx7625_data *ctx)
{
int ret;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
/* Select HDCP 1.4 KEY */
ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
@@ -843,7 +843,7 @@ static int anx7625_hdcp_key_load(struct anx7625_data *ctx)
static int anx7625_hdcp_disable(struct anx7625_data *ctx)
{
int ret;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
dev_dbg(dev, "disable HDCP 1.4\n");
@@ -864,7 +864,7 @@ static int anx7625_hdcp_enable(struct anx7625_data *ctx)
{
u8 bcap;
int ret;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
ret = anx7625_hdcp_key_probe(ctx);
if (ret) {
@@ -922,7 +922,7 @@ static int anx7625_hdcp_enable(struct anx7625_data *ctx)
static void anx7625_dp_start(struct anx7625_data *ctx)
{
int ret;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
u8 data;
if (!ctx->display_timing_valid) {
@@ -955,7 +955,7 @@ static void anx7625_dp_start(struct anx7625_data *ctx)
static void anx7625_dp_stop(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
int ret;
u8 data;
@@ -1020,7 +1020,7 @@ static int sp_tx_aux_rd(struct anx7625_data *ctx, u8 len_cmd)
static int sp_tx_get_edid_block(struct anx7625_data *ctx)
{
int c = 0;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
sp_tx_aux_wr(ctx, 0x7e);
sp_tx_aux_rd(ctx, 0x01);
@@ -1042,7 +1042,7 @@ static int edid_read(struct anx7625_data *ctx,
u8 offset, u8 *pblock_buf)
{
int ret, cnt;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
for (cnt = 0; cnt <= EDID_TRY_CNT; cnt++) {
sp_tx_aux_wr(ctx, offset);
@@ -1073,7 +1073,7 @@ static int segments_edid_read(struct anx7625_data *ctx,
{
u8 cnt;
int ret;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
/* Write address only */
ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
@@ -1128,7 +1128,7 @@ static int sp_tx_edid_read(struct anx7625_data *ctx,
u8 i, j;
int g_edid_break = 0;
int ret;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
/* Address initial */
ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
@@ -1235,7 +1235,7 @@ static int sp_tx_edid_read(struct anx7625_data *ctx,
static void anx7625_power_on(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
int ret, i;
if (!ctx->pdata.low_power_mode) {
@@ -1271,7 +1271,7 @@ static void anx7625_power_on(struct anx7625_data *ctx)
static void anx7625_power_standby(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
int ret;
if (!ctx->pdata.low_power_mode) {
@@ -1301,7 +1301,7 @@ static void anx7625_config(struct anx7625_data *ctx)
static void anx7625_disable_pd_protocol(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
int ret;
/* Reset main ocm */
@@ -1321,7 +1321,7 @@ static void anx7625_disable_pd_protocol(struct anx7625_data *ctx)
static int anx7625_ocm_loading_check(struct anx7625_data *ctx)
{
int ret;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
/* Check interface workable */
ret = anx7625_reg_read(ctx, ctx->i2c.rx_p0_client,
@@ -1367,7 +1367,7 @@ static void anx7625_power_on_init(struct anx7625_data *ctx)
static void anx7625_init_gpio(struct anx7625_data *platform)
{
- struct device *dev = &platform->client->dev;
+ struct device *dev = platform->dev;
DRM_DEV_DEBUG_DRIVER(dev, "init gpio\n");
@@ -1407,7 +1407,7 @@ static void anx7625_stop_dp_work(struct anx7625_data *ctx)
static void anx7625_start_dp_work(struct anx7625_data *ctx)
{
int ret;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
if (ctx->hpd_high_cnt >= 2) {
DRM_DEV_DEBUG_DRIVER(dev, "filter useless HPD\n");
@@ -1459,7 +1459,7 @@ static int _anx7625_hpd_polling(struct anx7625_data *ctx,
unsigned long wait_us)
{
int ret, val;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
/* Interrupt mode, no need poll HPD status, just return */
if (ctx->pdata.intp_irq)
@@ -1493,7 +1493,7 @@ static int anx7625_wait_hpd_asserted(struct drm_dp_aux *aux,
unsigned long wait_us)
{
struct anx7625_data *ctx = container_of(aux, struct anx7625_data, aux);
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
int ret;
pm_runtime_get_sync(dev);
@@ -1526,7 +1526,7 @@ static void anx7625_dp_adjust_swing(struct anx7625_data *ctx)
static void dp_hpd_change_handler(struct anx7625_data *ctx, bool on)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
/* HPD changed */
DRM_DEV_DEBUG_DRIVER(dev, "dp_hpd_change_default_func: %d\n",
@@ -1546,7 +1546,7 @@ static void dp_hpd_change_handler(struct anx7625_data *ctx, bool on)
static int anx7625_hpd_change_detect(struct anx7625_data *ctx)
{
int intr_vector, status;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
status = anx7625_reg_write(ctx, ctx->i2c.tcpc_client,
INTR_ALERT_1, 0xFF);
@@ -1594,7 +1594,7 @@ static void anx7625_work_func(struct work_struct *work)
mutex_lock(&ctx->lock);
- if (pm_runtime_suspended(&ctx->client->dev))
+ if (pm_runtime_suspended(ctx->dev))
goto unlock;
event = anx7625_hpd_change_detect(ctx);
@@ -1738,7 +1738,7 @@ static ssize_t anx7625_aux_transfer(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
{
struct anx7625_data *ctx = container_of(aux, struct anx7625_data, aux);
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
u8 request = msg->request & ~DP_AUX_I2C_MOT;
int ret = 0;
@@ -1764,7 +1764,7 @@ static ssize_t anx7625_aux_transfer(struct drm_dp_aux *aux,
static struct edid *anx7625_get_edid(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
struct s_edid_data *p_edid = &ctx->slimport_edid_p;
int edid_num;
u8 *edid;
@@ -1800,7 +1800,7 @@ static struct edid *anx7625_get_edid(struct anx7625_data *ctx)
static enum drm_connector_status anx7625_sink_detect(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
DRM_DEV_DEBUG_DRIVER(dev, "sink detect\n");
@@ -2009,7 +2009,7 @@ static const struct hdmi_codec_ops anx7625_codec_ops = {
static void anx7625_unregister_audio(struct anx7625_data *ctx)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
if (ctx->audio_pdev) {
platform_device_unregister(ctx->audio_pdev);
@@ -2045,7 +2045,7 @@ static int anx7625_register_audio(struct device *dev, struct anx7625_data *ctx)
static int anx7625_attach_dsi(struct anx7625_data *ctx)
{
struct mipi_dsi_device *dsi;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
struct mipi_dsi_host *host;
const struct mipi_dsi_device_info info = {
.type = "anx7625",
@@ -2098,7 +2098,7 @@ static void hdcp_check_work_func(struct work_struct *work)
dwork = to_delayed_work(work);
ctx = container_of(dwork, struct anx7625_data, hdcp_work);
- dev = &ctx->client->dev;
+ dev = ctx->dev;
if (!ctx->connector) {
dev_err(dev, "HDCP connector is null!");
@@ -2125,7 +2125,7 @@ static void hdcp_check_work_func(struct work_struct *work)
static int anx7625_connector_atomic_check(struct anx7625_data *ctx,
struct drm_connector_state *state)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
int cp;
dev_dbg(dev, "hdcp state check\n");
@@ -2170,7 +2170,7 @@ static int anx7625_bridge_attach(struct drm_bridge *bridge,
{
struct anx7625_data *ctx = bridge_to_anx7625(bridge);
int err;
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
DRM_DEV_DEBUG_DRIVER(dev, "drm attach\n");
if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
@@ -2214,7 +2214,7 @@ anx7625_bridge_mode_valid(struct drm_bridge *bridge,
const struct drm_display_mode *mode)
{
struct anx7625_data *ctx = bridge_to_anx7625(bridge);
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
DRM_DEV_DEBUG_DRIVER(dev, "drm mode checking\n");
@@ -2235,7 +2235,7 @@ static void anx7625_bridge_mode_set(struct drm_bridge *bridge,
const struct drm_display_mode *mode)
{
struct anx7625_data *ctx = bridge_to_anx7625(bridge);
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
DRM_DEV_DEBUG_DRIVER(dev, "drm mode set\n");
@@ -2281,7 +2281,7 @@ static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge,
struct drm_display_mode *adj)
{
struct anx7625_data *ctx = bridge_to_anx7625(bridge);
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
u32 hsync, hfp, hbp, hblanking;
u32 adj_hsync, adj_hfp, adj_hbp, adj_hblanking, delta_adj;
u32 vref, adj_clock;
@@ -2399,7 +2399,7 @@ static int anx7625_bridge_atomic_check(struct drm_bridge *bridge,
struct drm_connector_state *conn_state)
{
struct anx7625_data *ctx = bridge_to_anx7625(bridge);
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
dev_dbg(dev, "drm bridge atomic check\n");
@@ -2413,7 +2413,7 @@ static void anx7625_bridge_atomic_enable(struct drm_bridge *bridge,
struct drm_bridge_state *state)
{
struct anx7625_data *ctx = bridge_to_anx7625(bridge);
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
struct drm_connector *connector;
dev_dbg(dev, "drm atomic enable\n");
@@ -2440,7 +2440,7 @@ static void anx7625_bridge_atomic_disable(struct drm_bridge *bridge,
struct drm_bridge_state *old)
{
struct anx7625_data *ctx = bridge_to_anx7625(bridge);
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
dev_dbg(dev, "drm atomic disable\n");
@@ -2454,7 +2454,7 @@ static enum drm_connector_status
anx7625_bridge_detect(struct drm_bridge *bridge)
{
struct anx7625_data *ctx = bridge_to_anx7625(bridge);
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
DRM_DEV_DEBUG_DRIVER(dev, "drm bridge detect\n");
@@ -2465,7 +2465,7 @@ static struct edid *anx7625_bridge_get_edid(struct drm_bridge *bridge,
struct drm_connector *connector)
{
struct anx7625_data *ctx = bridge_to_anx7625(bridge);
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
DRM_DEV_DEBUG_DRIVER(dev, "drm bridge get edid\n");
@@ -2490,7 +2490,7 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
struct i2c_client *client)
{
- struct device *dev = &ctx->client->dev;
+ struct device *dev = ctx->dev;
ctx->i2c.tx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
TX_P0_ADDR >> 1);
@@ -2591,7 +2591,7 @@ static int anx7625_i2c_probe(struct i2c_client *client)
pdata = &platform->pdata;
- platform->client = client;
+ platform->dev = &client->dev;
i2c_set_clientdata(client, platform);
pdata->supplies[0].supply = "vdd10";
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 14f33d6be289..5af819611ebc 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -458,7 +458,7 @@ struct anx7625_data {
int hdcp_cp;
/* Lock for work queue */
struct mutex lock;
- struct i2c_client *client;
+ struct device *dev;
struct anx7625_i2c_client i2c;
struct i2c_client *last_client;
struct timer_list hdcp_timer;
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index abaf6e23775e..39edcbad586f 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -404,7 +404,7 @@ struct debugfs_entries {
struct it6505 {
struct drm_dp_aux aux;
struct drm_bridge bridge;
- struct i2c_client *client;
+ struct device *dev;
struct it6505_drm_dp_link link;
struct it6505_platform_data pdata;
/*
@@ -524,7 +524,7 @@ static int it6505_read(struct it6505 *it6505, unsigned int reg_addr)
{
unsigned int value;
int err;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
if (!it6505->powered)
return -ENODEV;
@@ -542,7 +542,7 @@ static int it6505_write(struct it6505 *it6505, unsigned int reg_addr,
unsigned int reg_val)
{
int err;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
if (!it6505->powered)
return -ENODEV;
@@ -562,7 +562,7 @@ static int it6505_set_bits(struct it6505 *it6505, unsigned int reg,
unsigned int mask, unsigned int value)
{
int err;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
if (!it6505->powered)
return -ENODEV;
@@ -580,7 +580,7 @@ static int it6505_set_bits(struct it6505 *it6505, unsigned int reg,
static void it6505_debug_print(struct it6505 *it6505, unsigned int reg,
const char *prefix)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int val;
if (!drm_debug_enabled(DRM_UT_DRIVER))
@@ -599,7 +599,7 @@ static int it6505_dpcd_read(struct it6505 *it6505, unsigned long offset)
{
u8 value;
int ret;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
ret = drm_dp_dpcd_readb(&it6505->aux, offset, &value);
if (ret < 0) {
@@ -613,7 +613,7 @@ static int it6505_dpcd_write(struct it6505 *it6505, unsigned long offset,
u8 datain)
{
int ret;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
ret = drm_dp_dpcd_writeb(&it6505->aux, offset, datain);
if (ret < 0) {
@@ -626,7 +626,7 @@ static int it6505_dpcd_write(struct it6505 *it6505, unsigned long offset,
static int it6505_get_dpcd(struct it6505 *it6505, int offset, u8 *dpcd, int num)
{
int ret;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
ret = drm_dp_dpcd_read(&it6505->aux, offset, dpcd, num);
@@ -643,7 +643,7 @@ static void it6505_dump(struct it6505 *it6505)
{
unsigned int i, j;
u8 regs[16];
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
for (i = 0; i <= 0xff; i += 16) {
for (j = 0; j < 16; j++)
@@ -682,7 +682,7 @@ static int it6505_read_word(struct it6505 *it6505, unsigned int reg)
static void it6505_calc_video_info(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int hsync_pol, vsync_pol, interlaced;
int htotal, hdes, hdew, hfph, hsyncw;
int vtotal, vdes, vdew, vfph, vsyncw;
@@ -926,7 +926,7 @@ static int it6505_aux_wait(struct it6505 *it6505)
{
int status;
unsigned long timeout;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
timeout = jiffies + msecs_to_jiffies(AUX_WAIT_TIMEOUT_MS) + 1;
@@ -1141,7 +1141,7 @@ static int it6505_get_edid_block(void *data, u8 *buf, unsigned int block,
size_t len)
{
struct it6505 *it6505 = data;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
enum aux_cmd_reply reply;
int offset, ret, aux_retry = 100;
@@ -1201,7 +1201,7 @@ static int it6505_send_video_infoframe(struct it6505 *it6505,
{
u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
int err;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
err = hdmi_avi_infoframe_pack(frame, buffer, sizeof(buffer));
if (err < 0) {
@@ -1231,7 +1231,7 @@ static void it6505_get_extcon_property(struct it6505 *it6505)
{
int err;
union extcon_property_value property;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
if (it6505->extcon && !it6505->lane_swap_disabled) {
err = extcon_get_property(it6505->extcon, EXTCON_DISP_DP,
@@ -1382,7 +1382,7 @@ static void it6505_enable_audio_source(struct it6505 *it6505)
static void it6505_enable_audio_infoframe(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
u8 audio_info_ca[] = { 0x00, 0x00, 0x01, 0x03, 0x07, 0x0B, 0x0F, 0x1F };
DRM_DEV_DEBUG_DRIVER(dev, "infoframe channel_allocation:0x%02x",
@@ -1411,7 +1411,7 @@ static void it6505_disable_audio(struct it6505 *it6505)
static void it6505_enable_audio(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int regbe;
DRM_DEV_DEBUG_DRIVER(dev, "start");
@@ -1446,7 +1446,7 @@ static bool it6505_use_step_train_check(struct it6505 *it6505)
static void it6505_parse_link_capabilities(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
struct it6505_drm_dp_link *link = &it6505->link;
int bcaps;
@@ -1557,7 +1557,7 @@ static void it6505_lane_count_setup(struct it6505 *it6505)
static void it6505_link_training_setup(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
if (it6505->enable_enhanced_frame)
it6505_set_bits(it6505, REG_DATA_MUTE_CTRL,
@@ -1708,7 +1708,7 @@ it6505_step_cr_train(struct it6505 *it6505,
FORCE_CR_DONE);
return true;
}
- DRM_DEV_DEBUG_DRIVER(&it6505->client->dev, "cr not done");
+ DRM_DEV_DEBUG_DRIVER(it6505->dev, "cr not done");
if (it6505_check_max_voltage_swing_reached(lane_level_config,
it6505->lane_count))
@@ -1785,7 +1785,7 @@ it6505_step_eq_train(struct it6505 *it6505,
FORCE_EQ_DONE);
return true;
}
- DRM_DEV_DEBUG_DRIVER(&it6505->client->dev, "eq not done");
+ DRM_DEV_DEBUG_DRIVER(it6505->dev, "eq not done");
for (i = 0; i < it6505->lane_count; i++) {
lane_voltage_pre_emphasis->voltage_swing[i] =
@@ -1820,7 +1820,7 @@ static bool it6505_link_start_step_train(struct it6505 *it6505)
.pre_emphasis = { 0 },
};
- DRM_DEV_DEBUG_DRIVER(&it6505->client->dev, "start");
+ DRM_DEV_DEBUG_DRIVER(it6505->dev, "start");
err = it6505_drm_dp_link_configure(it6505);
if (err < 0)
@@ -1854,7 +1854,7 @@ static void it6505_reset_hdcp(struct it6505 *it6505)
static void it6505_start_hdcp(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "start");
it6505_reset_hdcp(it6505);
@@ -1882,7 +1882,7 @@ static bool it6505_hdcp_is_ksv_valid(u8 *ksv)
static void it6505_hdcp_part1_auth(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
u8 hdcp_bcaps;
it6505_set_bits(it6505, REG_RESET_CTRL, HDCP_RESET, 0x00);
@@ -1923,7 +1923,7 @@ static int it6505_sha1_digest(struct it6505 *it6505, u8 *sha1_input,
struct shash_desc *desc;
struct crypto_shash *tfm;
int err;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
tfm = crypto_alloc_shash("sha1", 0, 0);
if (IS_ERR(tfm)) {
@@ -1948,7 +1948,7 @@ static int it6505_sha1_digest(struct it6505 *it6505, u8 *sha1_input,
static int it6505_setup_sha1_input(struct it6505 *it6505, u8 *sha1_input)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
u8 binfo[2];
int down_stream_count, i, err, msg_count = 0;
@@ -2012,7 +2012,7 @@ static int it6505_setup_sha1_input(struct it6505 *it6505, u8 *sha1_input)
static bool it6505_hdcp_part2_ksvlist_check(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
u8 av[5][4], bv[5][4];
int i, err;
@@ -2045,7 +2045,7 @@ static void it6505_hdcp_wait_ksv_list(struct work_struct *work)
{
struct it6505 *it6505 = container_of(work, struct it6505,
hdcp_wait_ksv_list);
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
unsigned int timeout = 5000;
u8 bstatus = 0;
bool ksv_list_check;
@@ -2087,7 +2087,7 @@ static void it6505_hdcp_work(struct work_struct *work)
{
struct it6505 *it6505 = container_of(work, struct it6505,
hdcp_work.work);
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int ret;
u8 link_status[DP_LINK_STATUS_SIZE] = { 0 };
@@ -2128,7 +2128,7 @@ static void it6505_hdcp_work(struct work_struct *work)
static void it6505_show_hdcp_info(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int i;
u8 *sha1 = it6505->sha1_input;
@@ -2162,7 +2162,7 @@ static void it6505_stop_link_train(struct it6505 *it6505)
static void it6505_link_train_ok(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
it6505->link_state = LINK_OK;
/* disalbe mute enable avi info frame */
@@ -2181,7 +2181,7 @@ static void it6505_link_train_ok(struct it6505 *it6505)
static void it6505_link_step_train_process(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int ret, i, step_retry = 3;
DRM_DEV_DEBUG_DRIVER(dev, "Start step train");
@@ -2219,7 +2219,7 @@ static void it6505_link_step_train_process(struct it6505 *it6505)
static void it6505_link_training_work(struct work_struct *work)
{
struct it6505 *it6505 = container_of(work, struct it6505, link_works);
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int ret;
DRM_DEV_DEBUG_DRIVER(dev, "it6505->sink_count: %d",
@@ -2267,7 +2267,7 @@ static void it6505_remove_edid(struct it6505 *it6505)
static int it6505_process_hpd_irq(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int ret, dpcd_sink_count, dp_irq_vector, bstatus;
u8 link_status[DP_LINK_STATUS_SIZE];
@@ -2331,7 +2331,7 @@ static int it6505_process_hpd_irq(struct it6505 *it6505)
static void it6505_irq_hpd(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int dp_sink_count;
it6505->hpd_state = it6505_get_sink_hpd_status(it6505);
@@ -2393,7 +2393,7 @@ static void it6505_irq_hpd(struct it6505 *it6505)
static void it6505_irq_hpd_irq(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "hpd_irq interrupt");
@@ -2403,7 +2403,7 @@ static void it6505_irq_hpd_irq(struct it6505 *it6505)
static void it6505_irq_scdt(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
bool data;
data = it6505_get_video_status(it6505);
@@ -2418,7 +2418,7 @@ static void it6505_irq_scdt(struct it6505 *it6505)
static void it6505_irq_hdcp_done(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "hdcp done interrupt");
it6505->hdcp_status = HDCP_AUTH_DONE;
@@ -2427,7 +2427,7 @@ static void it6505_irq_hdcp_done(struct it6505 *it6505)
static void it6505_irq_hdcp_fail(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "hdcp fail interrupt");
it6505->hdcp_status = HDCP_AUTH_IDLE;
@@ -2437,14 +2437,14 @@ static void it6505_irq_hdcp_fail(struct it6505 *it6505)
static void it6505_irq_aux_cmd_fail(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "AUX PC Request Fail Interrupt");
}
static void it6505_irq_hdcp_ksv_check(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "HDCP event Interrupt");
schedule_work(&it6505->hdcp_wait_ksv_list);
@@ -2452,7 +2452,7 @@ static void it6505_irq_hdcp_ksv_check(struct it6505 *it6505)
static void it6505_irq_audio_fifo_error(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "audio fifo error Interrupt");
@@ -2462,7 +2462,7 @@ static void it6505_irq_audio_fifo_error(struct it6505 *it6505)
static void it6505_irq_link_train_fail(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "link training fail interrupt");
schedule_work(&it6505->link_works);
@@ -2470,7 +2470,7 @@ static void it6505_irq_link_train_fail(struct it6505 *it6505)
static void it6505_irq_video_fifo_error(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "video fifo overflow interrupt");
it6505->auto_train_retry = AUTO_TRAIN_RETRY;
@@ -2481,7 +2481,7 @@ static void it6505_irq_video_fifo_error(struct it6505 *it6505)
static void it6505_irq_io_latch_fifo_overflow(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "IO latch fifo overflow interrupt");
it6505->auto_train_retry = AUTO_TRAIN_RETRY;
@@ -2498,7 +2498,7 @@ static bool it6505_test_bit(unsigned int bit, const unsigned int *addr)
static irqreturn_t it6505_int_threaded_handler(int unused, void *data)
{
struct it6505 *it6505 = data;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
static const struct {
int bit;
void (*handler)(struct it6505 *it6505);
@@ -2550,7 +2550,7 @@ static irqreturn_t it6505_int_threaded_handler(int unused, void *data)
static int it6505_poweron(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
struct it6505_platform_data *pdata = &it6505->pdata;
int err;
@@ -2599,7 +2599,7 @@ static int it6505_poweron(struct it6505 *it6505)
static int it6505_poweroff(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
struct it6505_platform_data *pdata = &it6505->pdata;
int err;
@@ -2633,7 +2633,7 @@ static int it6505_poweroff(struct it6505 *it6505)
static enum drm_connector_status it6505_detect(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
enum drm_connector_status status = connector_status_disconnected;
int dp_sink_count;
@@ -2694,7 +2694,7 @@ static int it6505_extcon_notifier(struct notifier_block *self,
static void it6505_extcon_work(struct work_struct *work)
{
struct it6505 *it6505 = container_of(work, struct it6505, extcon_wq);
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int state, ret;
if (it6505->enable_drv_hold)
@@ -2739,11 +2739,11 @@ static void it6505_extcon_work(struct work_struct *work)
static int it6505_use_notifier_module(struct it6505 *it6505)
{
int ret;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
it6505->event_nb.notifier_call = it6505_extcon_notifier;
INIT_WORK(&it6505->extcon_wq, it6505_extcon_work);
- ret = devm_extcon_register_notifier(&it6505->client->dev,
+ ret = devm_extcon_register_notifier(it6505->dev,
it6505->extcon, EXTCON_DISP_DP,
&it6505->event_nb);
if (ret) {
@@ -2759,7 +2759,7 @@ static int it6505_use_notifier_module(struct it6505 *it6505)
static void it6505_remove_notifier_module(struct it6505 *it6505)
{
if (it6505->extcon) {
- devm_extcon_unregister_notifier(&it6505->client->dev,
+ devm_extcon_unregister_notifier(it6505->dev,
it6505->extcon, EXTCON_DISP_DP,
&it6505->event_nb);
@@ -2772,7 +2772,7 @@ static void __maybe_unused it6505_delayed_audio(struct work_struct *work)
struct it6505 *it6505 = container_of(work, struct it6505,
delayed_audio.work);
- DRM_DEV_DEBUG_DRIVER(&it6505->client->dev, "start");
+ DRM_DEV_DEBUG_DRIVER(it6505->dev, "start");
if (!it6505->powered)
return;
@@ -2785,7 +2785,7 @@ static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505,
struct hdmi_codec_params
*params)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int i = 0;
DRM_DEV_DEBUG_DRIVER(dev, "%s %d Hz, %d bit, %d channels\n", __func__,
@@ -2869,7 +2869,7 @@ static int it6505_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)
{
struct it6505 *it6505 = bridge_to_it6505(bridge);
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
int ret;
if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
@@ -2933,7 +2933,7 @@ static void it6505_bridge_atomic_enable(struct drm_bridge *bridge,
struct drm_bridge_state *old_state)
{
struct it6505 *it6505 = bridge_to_it6505(bridge);
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
struct drm_atomic_state *state = old_state->base.state;
struct hdmi_avi_infoframe frame;
struct drm_crtc_state *crtc_state;
@@ -2989,7 +2989,7 @@ static void it6505_bridge_atomic_disable(struct drm_bridge *bridge,
struct drm_bridge_state *old_state)
{
struct it6505 *it6505 = bridge_to_it6505(bridge);
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "start");
@@ -3004,7 +3004,7 @@ static void it6505_bridge_atomic_pre_enable(struct drm_bridge *bridge,
struct drm_bridge_state *old_state)
{
struct it6505 *it6505 = bridge_to_it6505(bridge);
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "start");
@@ -3015,7 +3015,7 @@ static void it6505_bridge_atomic_post_disable(struct drm_bridge *bridge,
struct drm_bridge_state *old_state)
{
struct it6505 *it6505 = bridge_to_it6505(bridge);
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
DRM_DEV_DEBUG_DRIVER(dev, "start");
@@ -3034,7 +3034,7 @@ static struct edid *it6505_bridge_get_edid(struct drm_bridge *bridge,
struct drm_connector *connector)
{
struct it6505 *it6505 = bridge_to_it6505(bridge);
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
if (!it6505->cached_edid) {
it6505->cached_edid = drm_do_get_edid(connector, it6505_get_edid_block,
@@ -3086,7 +3086,7 @@ static const struct dev_pm_ops it6505_bridge_pm_ops = {
static int it6505_init_pdata(struct it6505 *it6505)
{
struct it6505_platform_data *pdata = &it6505->pdata;
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
/* 1.0V digital core power regulator */
pdata->pwr18 = devm_regulator_get(dev, "pwr18");
@@ -3128,7 +3128,7 @@ static int it6505_get_data_lanes_count(const struct device_node *endpoint,
static void it6505_parse_dt(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
struct device_node *np = dev->of_node, *ep = NULL;
int len;
u64 link_frequencies;
@@ -3332,7 +3332,7 @@ static void debugfs_create_files(struct it6505 *it6505)
static void debugfs_init(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
+ struct device *dev = it6505->dev;
it6505->debugfs = debugfs_create_dir(DEBUGFS_DIR_NAME, NULL);
@@ -3374,7 +3374,7 @@ static int it6505_i2c_probe(struct i2c_client *client)
it6505->bridge.of_node = client->dev.of_node;
it6505->connector_status = connector_status_disconnected;
- it6505->client = client;
+ it6505->dev = &client->dev;
i2c_set_clientdata(client, it6505);
/* get extcon device from DTS */
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v15 07/10] drm/bridge: anx7625: Register Type C mode switches
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
` (5 preceding siblings ...)
2023-03-31 9:11 ` [PATCH v15 06/10] drm/bridge: Remove redundant i2c_client in anx7625/it6505 Pin-yen Lin
@ 2023-03-31 9:11 ` Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 08/10] dt-bindings: display: bridge: it6505: Add mode-switch support Pin-yen Lin
` (2 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel
Register USB Type-C mode switches when the "mode-switch" property and
relevant ports are available in Device Tree. Configure the crosspoint
switch based on the entered alternate mode for a specific Type-C
connector.
Crosspoint switch can also be used for switching the output signal for
different orientations of a single USB Type-C connector, but the
orientation switch is not implemented yet. A TODO is added for this.
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
---
Changes in v15:
- Swap the definitions in anx7625_typec_port_data
- Add comments about the completion
- Abort the mux_set callback when the switch is unregistered
- Fix style issues
Changes in v14:
- Fix style issues
Changes in v12:
- Fixed style issues in anx7625 driver
- Fixed the inverted orientation setting in anx7625 driver
- Changed "&ctx->client->dev" to "ctx->dev"
- Fix style issues
- Updated the error logs when parsing data-lanes property
Changes in v11:
- Added back "data-lanes" parsing logics
- Removed Kconfig dependency
- Updated the usage of the private data
- Dropped Tested-by tag because of the new changes
Changes in v10:
- Added a TODO for implementing orientation switch for anx7625
- Updated the commit message for the absence of orientation switch
- Fixed typo in the commit message
- Collected Tested-by tag
Changes in v7:
- Fixed style issues in anx7625 driver
- Removed DT property validation in anx7625 driver.
- Extracted common codes to another commit.
Changes in v6:
- Squashed to a single patch
drivers/gpu/drm/bridge/analogix/anx7625.c | 157 ++++++++++++++++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 20 +++
2 files changed, 177 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 76d46db3f8dc..7ed5797e134c 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -15,6 +15,8 @@
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_mux.h>
#include <linux/workqueue.h>
#include <linux/of_gpio.h>
@@ -2570,6 +2572,154 @@ static void anx7625_runtime_disable(void *data)
pm_runtime_disable(data);
}
+static void anx7625_set_crosspoint_switch(struct anx7625_data *ctx,
+ enum typec_orientation orientation)
+{
+ if (orientation == TYPEC_ORIENTATION_NORMAL) {
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0,
+ SW_SEL1_SSRX_RX1 | SW_SEL1_DPTX0_RX2);
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1,
+ SW_SEL2_SSTX_TX1 | SW_SEL2_DPTX1_TX2);
+ } else if (orientation == TYPEC_ORIENTATION_REVERSE) {
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0,
+ SW_SEL1_SSRX_RX2 | SW_SEL1_DPTX0_RX1);
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1,
+ SW_SEL2_SSTX_TX2 | SW_SEL2_DPTX1_TX1);
+ }
+}
+
+static void anx7625_typec_two_ports_update(struct anx7625_data *ctx)
+{
+ unsigned int i;
+
+ /* Check if both ports available and do nothing to retain the current one */
+ if (ctx->port_data[0].dp_connected && ctx->port_data[1].dp_connected)
+ return;
+
+ for (i = 0; i < 2; i++) {
+ if (ctx->port_data[i].dp_connected)
+ anx7625_set_crosspoint_switch(ctx,
+ ctx->port_data[i].orientation);
+ }
+}
+
+static int anx7625_typec_mux_set(struct typec_mux_dev *mux,
+ struct typec_mux_state *state)
+{
+ struct drm_dp_typec_port_data *port = typec_mux_get_drvdata(mux);
+ struct anx7625_data *ctx = port->data;
+ struct device *dev = ctx->dev;
+ struct drm_dp_typec_switch_desc switch_desc = ctx->switch_desc;
+ bool new_dp_connected, old_dp_connected;
+
+ if (switch_desc.num_typec_switches == 1)
+ return 0;
+
+ /*
+ * The completion is called in anx7625_register_typec_switches
+ * even on errors, so it's safe to wait for completion without timeout.
+ */
+ wait_for_completion(&ctx->mux_register);
+
+ /* Abort when the switches are unregistered. */
+ if (!ctx->port_data)
+ return -EINVAL;
+
+ old_dp_connected = ctx->port_data[0].dp_connected ||
+ ctx->port_data[1].dp_connected;
+
+ ctx->port_data[port->port_num].dp_connected =
+ state->alt &&
+ state->alt->svid == USB_TYPEC_DP_SID &&
+ state->alt->mode == USB_TYPEC_DP_MODE;
+
+ dev_dbg(dev, "mux_set dp_connected: c0=%d, c1=%d\n",
+ ctx->port_data[0].dp_connected, ctx->port_data[1].dp_connected);
+
+ new_dp_connected = ctx->port_data[0].dp_connected ||
+ ctx->port_data[1].dp_connected;
+
+ /* DP on, power on first */
+ if (!old_dp_connected && new_dp_connected)
+ pm_runtime_get_sync(dev);
+
+ anx7625_typec_two_ports_update(ctx);
+
+ /* DP off, power off last */
+ if (old_dp_connected && !new_dp_connected)
+ pm_runtime_put_sync(dev);
+
+ return 0;
+}
+
+static void anx7625_unregister_typec_switches(struct anx7625_data *ctx)
+{
+ drm_dp_unregister_typec_switches(&ctx->switch_desc);
+ ctx->port_data = NULL;
+}
+
+static int anx7625_register_typec_switches(struct device *dev, struct anx7625_data *ctx)
+{
+ struct device_node *port_node = of_graph_get_port_by_id(dev->of_node, 1);
+ struct drm_dp_typec_switch_desc *switch_desc = &ctx->switch_desc;
+ int ret;
+ u32 dp_lanes[4];
+ unsigned int i, num_lanes;
+
+ /*
+ * Currently, only mode switch is implemented.
+ * TODO: Implement Type-C orientation switch for anx7625.
+ */
+ ret = drm_dp_register_typec_switches(dev, &port_node->fwnode,
+ &ctx->switch_desc, ctx,
+ anx7625_typec_mux_set);
+ if (ret)
+ return ret;
+
+ ctx->port_data = devm_kcalloc(dev, switch_desc->num_typec_switches,
+ sizeof(*ctx->port_data),
+ GFP_KERNEL);
+ if (!ctx->port_data) {
+ ret = -ENOMEM;
+ goto unregister_mux;
+ }
+
+ for (i = 0; i < switch_desc->num_typec_switches; i++) {
+ struct drm_dp_typec_port_data *port = &switch_desc->typec_ports[i];
+ struct fwnode_handle *fwnode = port->fwnode;
+
+ ret = fwnode_property_count_u32(fwnode, "data-lanes");
+ if (ret < 0) {
+ dev_err(dev,
+ "Error on getting data lanes count from %pfwP: %d\n",
+ fwnode, ret);
+ goto unregister_mux;
+ }
+ num_lanes = ret;
+
+ ret = fwnode_property_read_u32_array(fwnode, "data-lanes",
+ dp_lanes, num_lanes);
+ if (ret) {
+ dev_err(dev,
+ "Failed to read the data-lanes variable: %d\n",
+ ret);
+ goto unregister_mux;
+ }
+
+ ctx->port_data[i].orientation = dp_lanes[0] < 2 ?
+ TYPEC_ORIENTATION_REVERSE : TYPEC_ORIENTATION_NORMAL;
+ ctx->port_data[i].dp_connected = false;
+ }
+ complete_all(&ctx->mux_register);
+
+ return 0;
+
+unregister_mux:
+ anx7625_unregister_typec_switches(ctx);
+ complete_all(&ctx->mux_register);
+ return ret;
+}
+
static int anx7625_i2c_probe(struct i2c_client *client)
{
struct anx7625_data *platform;
@@ -2607,6 +2757,7 @@ static int anx7625_i2c_probe(struct i2c_client *client)
mutex_init(&platform->lock);
mutex_init(&platform->hdcp_wq_lock);
+ init_completion(&platform->mux_register);
INIT_DELAYED_WORK(&platform->hdcp_work, hdcp_check_work_func);
platform->hdcp_workqueue = create_workqueue("hdcp workqueue");
@@ -2677,6 +2828,10 @@ static int anx7625_i2c_probe(struct i2c_client *client)
if (platform->pdata.intp_irq)
queue_work(platform->workqueue, &platform->work);
+ ret = anx7625_register_typec_switches(dev, platform);
+ if (ret && ret != -ENODEV)
+ dev_warn(dev, "Didn't register Type-C switches, err: %d\n", ret);
+
platform->bridge.funcs = &anx7625_bridge_funcs;
platform->bridge.of_node = client->dev.of_node;
if (!anx7625_of_panel_on_aux_bus(&client->dev))
@@ -2728,6 +2883,8 @@ static void anx7625_i2c_remove(struct i2c_client *client)
drm_bridge_remove(&platform->bridge);
+ anx7625_unregister_typec_switches(platform);
+
if (platform->pdata.intp_irq)
destroy_workqueue(platform->workqueue);
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 5af819611ebc..291dba62e096 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -55,6 +55,18 @@
#define HPD_STATUS_CHANGE 0x80
#define HPD_STATUS 0x80
+#define TCPC_SWITCH_0 0xB4
+#define SW_SEL1_DPTX0_RX2 BIT(0)
+#define SW_SEL1_DPTX0_RX1 BIT(1)
+#define SW_SEL1_SSRX_RX2 BIT(4)
+#define SW_SEL1_SSRX_RX1 BIT(5)
+
+#define TCPC_SWITCH_1 0xB5
+#define SW_SEL2_DPTX1_TX2 BIT(0)
+#define SW_SEL2_DPTX1_TX1 BIT(1)
+#define SW_SEL2_SSTX_TX2 BIT(4)
+#define SW_SEL2_SSTX_TX1 BIT(5)
+
/******** END of I2C Address 0x58 ********/
/***************************************************************/
@@ -449,6 +461,11 @@ struct anx7625_i2c_client {
struct i2c_client *tcpc_client;
};
+struct anx7625_typec_port_data {
+ enum typec_orientation orientation;
+ bool dp_connected;
+};
+
struct anx7625_data {
struct anx7625_platform_data pdata;
struct platform_device *audio_pdev;
@@ -479,6 +496,9 @@ struct anx7625_data {
struct drm_connector *connector;
struct mipi_dsi_device *dsi;
struct drm_dp_aux aux;
+ struct completion mux_register;
+ struct drm_dp_typec_switch_desc switch_desc;
+ struct anx7625_typec_port_data *port_data;
};
#endif /* __ANX7625_H__ */
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v15 08/10] dt-bindings: display: bridge: it6505: Add mode-switch support
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
` (6 preceding siblings ...)
2023-03-31 9:11 ` [PATCH v15 07/10] drm/bridge: anx7625: Register Type C mode switches Pin-yen Lin
@ 2023-03-31 9:11 ` Pin-yen Lin
2023-04-06 15:15 ` Rob Herring
2023-03-31 9:11 ` [PATCH v15 09/10] drm/bridge: it6505: Fix Kconfig indentation Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 10/10] drm/bridge: it6505: Register Type C mode switches Pin-yen Lin
9 siblings, 1 reply; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel, Allen Chen
ITE IT6505 can be used in systems to switch the DP traffic between
two downstreams, which can be USB Type-C DisplayPort alternate mode
lane or regular DisplayPort output ports.
Update the binding to accommodate this usage by introducing a
data-lanes and a mode-switch property on endpoints.
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
---
(no changes since v12)
Changes in v12:
- Fixed the schema of "data-lanes" property for it6505
- Reworded the description of the mode-switch property
Changes in v11:
- Updated the description of the endpoints in the bindings
- Referenced video-interfaces.yaml instead for the endpoints binding
- Removed duplicated definitions from inherited schema
Changes in v9:
- Fixed subject prefix again
- Changed the naming of the example node for it6505
Changes in v8:
- Updated bindings for data-lanes property
- Fixed subject prefix
Changes in v7:
- Fixed issues reported by dt_binding_check.
- Updated the schema and the example dts for data-lanes.
- Changed to generic naming for the example dts node.
Changes in v6:
- Remove switches node and use endpoints and data-lanes property to
describe the connections.
.../bindings/display/bridge/ite,it6505.yaml | 101 +++++++++++++++---
1 file changed, 88 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
index c9a882ee6d98..348b02f26041 100644
--- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
@@ -75,22 +75,49 @@ properties:
port@1:
$ref: /schemas/graph.yaml#/$defs/port-base
unevaluatedProperties: false
- description: Video port for DP output
+ description:
+ Video port for DP output. Each endpoint connects to a video output
+ downstream, and the "data-lanes" property is used to describe the pin
+ connections. 0, 1, 2, 3 in "data-lanes" maps to TX0, TX1, TX2, TX3,
+ respectively.
- properties:
- endpoint:
- $ref: /schemas/graph.yaml#/$defs/endpoint-base
+
+ patternProperties:
+ "^endpoint@[01]$":
+ $ref: /schemas/media/video-interfaces.yaml#
unevaluatedProperties: false
properties:
+ reg: true
+
+ remote-endpoint: true
+
data-lanes:
- minItems: 1
- uniqueItems: true
- items:
- - enum: [ 0, 1 ]
- - const: 1
- - const: 2
- - const: 3
+ oneOf:
+ - items:
+ - enum: [0, 3]
+
+ - items:
+ - const: 0
+ - const: 1
+
+ - items:
+ - const: 3
+ - const: 2
+
+ - items:
+ - const: 0
+ - const: 1
+ - const: 2
+ - const: 3
+
+ mode-switch:
+ type: boolean
+ description: Serves as Type-C mode switch if present.
+
+ required:
+ - reg
+ - remote-endpoint
required:
- port@0
@@ -102,7 +129,6 @@ required:
- pwr18-supply
- interrupts
- reset-gpios
- - extcon
- ports
additionalProperties: false
@@ -139,8 +165,11 @@ examples:
};
port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
reg = <1>;
- it6505_out: endpoint {
+ it6505_out: endpoint@0 {
+ reg = <0>;
remote-endpoint = <&dp_in>;
data-lanes = <0 1>;
};
@@ -148,3 +177,49 @@ examples:
};
};
};
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dp-bridge@5c {
+ compatible = "ite,it6505";
+ interrupts = <8 IRQ_TYPE_LEVEL_LOW 8 0>;
+ reg = <0x5c>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&it6505_pins>;
+ ovdd-supply = <&mt6366_vsim2_reg>;
+ pwr18-supply = <&pp1800_dpbrdg_dx>;
+ reset-gpios = <&pio 177 0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ it6505_dpi_in: endpoint {
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+ port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ ite_typec0: endpoint@0 {
+ reg = <0>;
+ mode-switch;
+ data-lanes = <0 1>;
+ remote-endpoint = <&typec_port0>;
+ };
+ ite_typec1: endpoint@1 {
+ reg = <1>;
+ mode-switch;
+ data-lanes = <3 2>;
+ remote-endpoint = <&typec_port1>;
+ };
+ };
+ };
+ };
+ };
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v15 09/10] drm/bridge: it6505: Fix Kconfig indentation
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
` (7 preceding siblings ...)
2023-03-31 9:11 ` [PATCH v15 08/10] dt-bindings: display: bridge: it6505: Add mode-switch support Pin-yen Lin
@ 2023-03-31 9:11 ` Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 10/10] drm/bridge: it6505: Register Type C mode switches Pin-yen Lin
9 siblings, 0 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel
Replace the spaces with tab characters in the Kconfig file.
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
(no changes since v10)
Changes in v10:
- Collected Reviewed-by tag
Changes in v7:
- New in v7
drivers/gpu/drm/bridge/Kconfig | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index f076a09afac0..8f81311710e5 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -74,19 +74,19 @@ config DRM_FSL_LDB
Support for i.MX8MP DPI-to-LVDS on-SoC encoder.
config DRM_ITE_IT6505
- tristate "ITE IT6505 DisplayPort bridge"
- depends on OF
+ tristate "ITE IT6505 DisplayPort bridge"
+ depends on OF
select DRM_DISPLAY_DP_HELPER
select DRM_DISPLAY_HDCP_HELPER
select DRM_DISPLAY_HELPER
- select DRM_DP_AUX_BUS
- select DRM_KMS_HELPER
- select DRM_DP_HELPER
- select EXTCON
- select CRYPTO
- select CRYPTO_HASH
- help
- ITE IT6505 DisplayPort bridge chip driver.
+ select DRM_DP_AUX_BUS
+ select DRM_KMS_HELPER
+ select DRM_DP_HELPER
+ select EXTCON
+ select CRYPTO
+ select CRYPTO_HASH
+ help
+ ITE IT6505 DisplayPort bridge chip driver.
config DRM_LONTIUM_LT8912B
tristate "Lontium LT8912B DSI/HDMI bridge"
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v15 10/10] drm/bridge: it6505: Register Type C mode switches
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
` (8 preceding siblings ...)
2023-03-31 9:11 ` [PATCH v15 09/10] drm/bridge: it6505: Fix Kconfig indentation Pin-yen Lin
@ 2023-03-31 9:11 ` Pin-yen Lin
9 siblings, 0 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-03-31 9:11 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
Pin-yen Lin, linux-kernel
Register USB Type-C mode switches when the "mode-switch" property and
relevant port are available in Device Tree. Configure the "lane_swap"
state based on the entered alternate mode for a specific Type-C
connector, which ends up updating the lane swap registers of the it6505
chip.
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
---
Changes in v15:
- Add comments about the completion
- Fix style issues
- Abort the mux_set callback when the switch is unregistered
Changes in v14:
- Fix style issues
Changes in v13:
- Fix style issues
Changes in v12:
- Fixes style issues in it6505 driver
- Replaced &it6505->client->dev with it6505->dev
- Updated the error logs when parsing data-lanes property
Changes in v11:
- Added back "data-lanes" parsing logics
- Removed Kconfig dependency
- Updated the usage of the private data
Changes in v7:
- Fixed style issues in it6505 driver
- Removed the redundant sleep in it6505 driver
- Removed DT property validation in it6505 driver
- Rebased to drm-misc-next
- Extracted common codes to another commit
Changes in v6:
- Changed it6505_typec_mux_set callback function to accommodate with
the latest drm-misc patches
- Changed the driver implementation to accommodate with the new binding
- Squashed to a single patch
drivers/gpu/drm/bridge/ite-it6505.c | 198 +++++++++++++++++++++++++++-
1 file changed, 191 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index 39edcbad586f..12fea5727c92 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -17,6 +17,8 @@
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/types.h>
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_mux.h>
#include <linux/wait.h>
#include <crypto/hash.h>
@@ -27,6 +29,7 @@
#include <drm/drm_bridge.h>
#include <drm/drm_crtc.h>
#include <drm/drm_edid.h>
+#include <drm/drm_of.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -401,6 +404,11 @@ struct debugfs_entries {
const struct file_operations *fops;
};
+struct it6505_typec_port_data {
+ bool dp_connected;
+ bool lane_swap;
+};
+
struct it6505 {
struct drm_dp_aux aux;
struct drm_bridge bridge;
@@ -454,6 +462,9 @@ struct it6505 {
struct delayed_work delayed_audio;
struct it6505_audio_data audio;
struct dentry *debugfs;
+ struct completion mux_register;
+ struct drm_dp_typec_switch_desc switch_desc;
+ struct it6505_typec_port_data *port_data;
/* it6505 driver hold option */
bool enable_drv_hold;
@@ -3357,12 +3368,172 @@ static void it6505_shutdown(struct i2c_client *client)
it6505_lane_off(it6505);
}
+static void it6505_typec_ports_update(struct it6505 *it6505)
+{
+ unsigned int i;
+
+ /* Check if both ports available and do nothing to retain the current one */
+ if (it6505->port_data[0].dp_connected && it6505->port_data[1].dp_connected)
+ return;
+
+ for (i = 0; i < 2; i++) {
+ if (it6505->port_data[i].dp_connected)
+ it6505->lane_swap = it6505->port_data[i].lane_swap;
+ }
+}
+
+static int it6505_typec_mux_set(struct typec_mux_dev *mux,
+ struct typec_mux_state *state)
+{
+ struct drm_dp_typec_port_data *port = typec_mux_get_drvdata(mux);
+ struct it6505 *it6505 = port->data;
+ struct device *dev = it6505->dev;
+ struct drm_dp_typec_switch_desc switch_desc = it6505->switch_desc;
+ bool old_dp_connected, new_dp_connected;
+ int ret = 0;
+
+ if (switch_desc.num_typec_switches == 1)
+ return 0;
+
+ mutex_lock(&it6505->extcon_lock);
+ /*
+ * The completion is called in it6505_register_typec_switches even on
+ * errors, so it's safe to wait for completion without timeout.
+ */
+ wait_for_completion(&it6505->mux_register);
+
+ /* Abort when the switches are unregistered. */
+ if (!it6505->port_data) {
+ ret = -EINVAL;
+ goto unlock;
+ }
+
+ old_dp_connected = it6505->port_data[0].dp_connected ||
+ it6505->port_data[1].dp_connected;
+
+ it6505->port_data[port->port_num].dp_connected =
+ state->alt &&
+ state->alt->svid == USB_TYPEC_DP_SID &&
+ state->alt->mode == USB_TYPEC_DP_MODE;
+
+ dev_dbg(dev, "mux_set dp_connected: c0=%d, c1=%d\n",
+ it6505->port_data[0].dp_connected, it6505->port_data[1].dp_connected);
+
+ new_dp_connected = it6505->port_data[0].dp_connected ||
+ it6505->port_data[1].dp_connected;
+
+ if (it6505->enable_drv_hold) {
+ dev_dbg(dev, "enable driver hold\n");
+ goto unlock;
+ }
+
+ it6505_typec_ports_update(it6505);
+
+ if (!old_dp_connected && new_dp_connected) {
+ /*
+ * pm_runtime_force_suspend() disables runtime PM when the
+ * system enters suspend state. But on system resume, mux_set
+ * can be triggered before pm_runtime_force_resume() re-enables
+ * runtime PM. This makes the bridge stay powered off if the
+ * downstream display is connected when the system is suspended.
+ * Handling the error here to make sure the bridge is powered
+ * on, and leave the PM runtime usage count incremented so
+ * the future runtime PM calls is balanced.
+ */
+ if (pm_runtime_get_sync(dev) < 0)
+ it6505_poweron(it6505);
+
+ complete_all(&it6505->extcon_completion);
+ }
+
+ if (old_dp_connected && !new_dp_connected) {
+ reinit_completion(&it6505->extcon_completion);
+ pm_runtime_put_sync(dev);
+ if (it6505->bridge.dev)
+ drm_helper_hpd_irq_event(it6505->bridge.dev);
+ memset(it6505->dpcd, 0, sizeof(it6505->dpcd));
+ }
+
+unlock:
+ mutex_unlock(&it6505->extcon_lock);
+ return ret;
+}
+
+static void it6505_unregister_typec_switches(struct it6505 *it6505)
+{
+ drm_dp_unregister_typec_switches(&it6505->switch_desc);
+ it6505->port_data = NULL;
+}
+
+static int it6505_register_typec_switches(struct device *dev, struct it6505 *it6505)
+{
+ struct device_node *port_node = of_graph_get_port_by_id(dev->of_node, 1);
+ struct drm_dp_typec_switch_desc *switch_desc = &it6505->switch_desc;
+ int ret;
+ u32 dp_lanes[4];
+ unsigned int i, num_lanes;
+
+ ret = drm_dp_register_typec_switches(dev, &port_node->fwnode,
+ &it6505->switch_desc, it6505,
+ it6505_typec_mux_set);
+ if (ret)
+ return ret;
+
+ it6505->port_data = devm_kcalloc(dev, switch_desc->num_typec_switches,
+ sizeof(*it6505->port_data),
+ GFP_KERNEL);
+ if (!it6505->port_data) {
+ ret = -ENOMEM;
+ goto unregister_mux;
+ }
+
+ for (i = 0; i < switch_desc->num_typec_switches; i++) {
+ struct drm_dp_typec_port_data *port = &switch_desc->typec_ports[i];
+ struct fwnode_handle *fwnode = port->fwnode;
+
+ ret = fwnode_property_count_u32(fwnode, "data-lanes");
+ if (ret < 0) {
+ dev_err(dev,
+ "Error on getting data lanes count from %pfwP: %d\n",
+ fwnode, ret);
+ goto unregister_mux;
+ }
+ if (ret > 2) {
+ dev_err(dev,
+ "Invalid data lanes count for mode switches from %pfwP: %d\n",
+ fwnode, ret);
+ ret = -EINVAL;
+ goto unregister_mux;
+ }
+ num_lanes = ret;
+
+ ret = fwnode_property_read_u32_array(fwnode, "data-lanes",
+ dp_lanes, num_lanes);
+ if (ret) {
+ dev_err(dev,
+ "Failed to read the data-lanes variable: %d\n",
+ ret);
+ goto unregister_mux;
+ }
+
+ it6505->port_data[i].lane_swap = (dp_lanes[0] >= 2);
+ }
+ complete_all(&it6505->mux_register);
+
+ return 0;
+
+unregister_mux:
+ it6505_unregister_typec_switches(it6505);
+ complete_all(&it6505->mux_register);
+ return ret;
+}
+
static int it6505_i2c_probe(struct i2c_client *client)
{
struct it6505 *it6505;
struct device *dev = &client->dev;
struct extcon_dev *extcon;
- int err, intp_irq;
+ int err, intp_irq, ret;
it6505 = devm_kzalloc(&client->dev, sizeof(*it6505), GFP_KERNEL);
if (!it6505)
@@ -3379,14 +3550,26 @@ static int it6505_i2c_probe(struct i2c_client *client)
/* get extcon device from DTS */
extcon = extcon_get_edev_by_phandle(dev, 0);
- if (PTR_ERR(extcon) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- if (IS_ERR(extcon)) {
- dev_err(dev, "can not get extcon device!");
- return PTR_ERR(extcon);
+ ret = PTR_ERR_OR_ZERO(extcon);
+ if (ret == -EPROBE_DEFER)
+ return ret;
+ if (ret) {
+ if (ret != -ENODEV)
+ dev_warn(dev, "Cannot get extcon device: %d\n", ret);
+
+ it6505->extcon = NULL;
+ } else {
+ it6505->extcon = extcon;
}
- it6505->extcon = extcon;
+ init_completion(&it6505->mux_register);
+ ret = it6505_register_typec_switches(dev, it6505);
+ if (ret != -ENODEV)
+ dev_warn(dev, "Didn't register Type-C switches, err: %d\n", ret);
+ if (ret && !it6505->extcon) {
+ dev_err(dev, "Both extcon and Type-C switch are not registered.\n");
+ return ret;
+ }
it6505->regmap = devm_regmap_init_i2c(client, &it6505_regmap_config);
if (IS_ERR(it6505->regmap)) {
@@ -3458,6 +3641,7 @@ static void it6505_i2c_remove(struct i2c_client *client)
it6505_debugfs_remove(it6505);
it6505_poweroff(it6505);
it6505_remove_edid(it6505);
+ it6505_unregister_typec_switches(it6505);
}
static const struct i2c_device_id it6505_id[] = {
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v15 08/10] dt-bindings: display: bridge: it6505: Add mode-switch support
2023-03-31 9:11 ` [PATCH v15 08/10] dt-bindings: display: bridge: it6505: Add mode-switch support Pin-yen Lin
@ 2023-04-06 15:15 ` Rob Herring
2023-04-06 15:50 ` Pin-yen Lin
0 siblings, 1 reply; 26+ messages in thread
From: Rob Herring @ 2023-04-06 15:15 UTC (permalink / raw)
To: Pin-yen Lin
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck,
Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Allen Chen
On Fri, Mar 31, 2023 at 05:11:43PM +0800, Pin-yen Lin wrote:
> ITE IT6505 can be used in systems to switch the DP traffic between
> two downstreams, which can be USB Type-C DisplayPort alternate mode
> lane or regular DisplayPort output ports.
>
> Update the binding to accommodate this usage by introducing a
> data-lanes and a mode-switch property on endpoints.
>
> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
>
> ---
>
> (no changes since v12)
>
> Changes in v12:
> - Fixed the schema of "data-lanes" property for it6505
> - Reworded the description of the mode-switch property
>
> Changes in v11:
> - Updated the description of the endpoints in the bindings
> - Referenced video-interfaces.yaml instead for the endpoints binding
> - Removed duplicated definitions from inherited schema
>
> Changes in v9:
> - Fixed subject prefix again
> - Changed the naming of the example node for it6505
>
> Changes in v8:
> - Updated bindings for data-lanes property
> - Fixed subject prefix
>
> Changes in v7:
> - Fixed issues reported by dt_binding_check.
> - Updated the schema and the example dts for data-lanes.
> - Changed to generic naming for the example dts node.
>
> Changes in v6:
> - Remove switches node and use endpoints and data-lanes property to
> describe the connections.
>
> .../bindings/display/bridge/ite,it6505.yaml | 101 +++++++++++++++---
> 1 file changed, 88 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
> index c9a882ee6d98..348b02f26041 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
> @@ -75,22 +75,49 @@ properties:
> port@1:
> $ref: /schemas/graph.yaml#/$defs/port-base
> unevaluatedProperties: false
> - description: Video port for DP output
> + description:
> + Video port for DP output. Each endpoint connects to a video output
> + downstream, and the "data-lanes" property is used to describe the pin
> + connections. 0, 1, 2, 3 in "data-lanes" maps to TX0, TX1, TX2, TX3,
> + respectively.
>
> - properties:
> - endpoint:
> - $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +
> + patternProperties:
> + "^endpoint@[01]$":
> + $ref: /schemas/media/video-interfaces.yaml#
> unevaluatedProperties: false
>
> properties:
> + reg: true
> +
> + remote-endpoint: true
You don't need to list these 2.
> +
> data-lanes:
> - minItems: 1
> - uniqueItems: true
> - items:
> - - enum: [ 0, 1 ]
> - - const: 1
> - - const: 2
> - - const: 3
> + oneOf:
> + - items:
> + - enum: [0, 3]
Only lane 1 wasn't valid? If so, mention that in the commit message.
> +
> + - items:
> + - const: 0
> + - const: 1
> +
> + - items:
> + - const: 3
> + - const: 2
> +
> + - items:
> + - const: 0
> + - const: 1
> + - const: 2
> + - const: 3
Isn't this the default if 'data-lanes' is omitted.
> +
> + mode-switch:
> + type: boolean
> + description: Serves as Type-C mode switch if present.
> +
> + required:
> + - reg
> + - remote-endpoint
required can be dropped.
>
> required:
> - port@0
> @@ -102,7 +129,6 @@ required:
> - pwr18-supply
> - interrupts
> - reset-gpios
> - - extcon
> - ports
>
> additionalProperties: false
> @@ -139,8 +165,11 @@ examples:
> };
>
> port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> reg = <1>;
> - it6505_out: endpoint {
> + it6505_out: endpoint@0 {
> + reg = <0>;
> remote-endpoint = <&dp_in>;
> data-lanes = <0 1>;
> };
> @@ -148,3 +177,49 @@ examples:
> };
> };
> };
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + i2c {
Why do we need a whole new example? Just extend the existing example
with new properties/nodes.
Rob
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-03-31 9:11 ` [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support Pin-yen Lin
@ 2023-04-06 15:18 ` Rob Herring
2023-04-12 1:38 ` Stephen Boyd
1 sibling, 0 replies; 26+ messages in thread
From: Rob Herring @ 2023-04-06 15:18 UTC (permalink / raw)
To: Pin-yen Lin
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck,
Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai
On Fri, Mar 31, 2023 at 05:11:39PM +0800, Pin-yen Lin wrote:
> Analogix 7625 can be used in systems to switch the DP traffic between
> two downstreams, which can be USB Type-C DisplayPort alternate mode
> lane or regular DisplayPort output ports.
>
> Update the binding to accommodate this usage by introducing a
> data-lanes and a mode-switch property on endpoints.
>
> Also include the link to the product brief in the bindings.
>
> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> ---
>
> (no changes since v12)
>
> Changes in v12:
> - Removed the 4-lane binding in analogix,anx7625.yaml
> - Reworded the description for the mode-switch property
>
> Changes in v11:
> - Updated the description of the endpoints
> - Referenced video-interfaces.yaml instead for the endpoints
>
> Changes in v10:
> - Collected Reviewed-by and Tested-by tags
>
> Changes in v9:
> - Collected Reviewed-by tag
>
> Changes in v8:
> - Updated anx7625 bindings for data-lane property
> - Fixed the subject prefix
>
> Changes in v7:
> - Fixed issues reported by dt_binding_check
> - Updated the schema and the example dts for data-lanes.
> - Changed to generic naming for the example dts node.
>
> Changes in v6:
> - Remove switches node and use endpoints and data-lanes property to
> describe the connections.
>
> .../display/bridge/analogix,anx7625.yaml | 88 ++++++++++++++++++-
> 1 file changed, 85 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index b42553ac505c..604c7391d74f 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -12,7 +12,8 @@ maintainers:
>
> description: |
> The ANX7625 is an ultra-low power 4K Mobile HD Transmitter
> - designed for portable devices.
> + designed for portable devices. Product brief is available at
> + https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf
>
> properties:
> compatible:
> @@ -112,9 +113,40 @@ properties:
> data-lanes: true
>
> port@1:
> - $ref: /schemas/graph.yaml#/properties/port
> + $ref: /schemas/graph.yaml#/$defs/port-base
> description:
> - Video port for panel or connector.
> + Video port for panel or connector. Each endpoint connects to a video
> + output downstream, and the "data-lanes" property is used to describe
> + the pin connections. 0, 1, 2, 3 in "data-lanes" maps to SSRX1, SSTX1,
> + SSRX2, SSTX2, respectively.
> +
> + patternProperties:
> + "^endpoint@[01]$":
> + $ref: /schemas/media/video-interfaces.yaml#
unevaluatedProperties: false
> + properties:
> + reg: true
> +
> + remote-endpoint: true
Don't need to list these 2.
> +
> + data-lanes:
> + oneOf:
> + - items:
> + - enum: [0, 1, 2, 3]
> +
> + - items:
> + - const: 0
> + - const: 1
> +
> + - items:
> + - const: 2
> + - const: 3
> +
> + mode-switch:
> + type: boolean
> + description: Serves as Type-C mode switch if present.
> +
> + required:
> + - remote-endpoint
Don't need required.
>
> required:
> - port@0
> @@ -186,3 +218,53 @@ examples:
> };
> };
> };
> + - |
> + i2c3 {
i2c-3
But better yet, just expand the existing example rather than a whole new
one.
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + encoder@58 {
> + compatible = "analogix,anx7625";
> + reg = <0x58>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&anx7625_dp_pins>;
> + enable-gpios = <&pio 176 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&pio 177 GPIO_ACTIVE_HIGH>;
> + vdd10-supply = <&pp1100_dpbrdg>;
> + vdd18-supply = <&pp1800_dpbrdg_dx>;
> + vdd33-supply = <&pp3300_dpbrdg_dx>;
> + analogix,audio-enable;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + anx7625_dp_in: endpoint {
> + bus-type = <7>;
> + remote-endpoint = <&dpi_out>;
> + };
> + };
> +
> + port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <1>;
> + anx_typec0: endpoint@0 {
> + reg = <0>;
> + mode-switch;
> + data-lanes = <0 1>;
> + remote-endpoint = <&typec_port0>;
> + };
> + anx_typec1: endpoint@1 {
> + reg = <1>;
> + mode-switch;
> + data-lanes = <2 3>;
> + remote-endpoint = <&typec_port1>;
> + };
> + };
> + };
> + };
> + };
> --
> 2.40.0.348.gf938b09366-goog
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 08/10] dt-bindings: display: bridge: it6505: Add mode-switch support
2023-04-06 15:15 ` Rob Herring
@ 2023-04-06 15:50 ` Pin-yen Lin
0 siblings, 0 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-04-06 15:50 UTC (permalink / raw)
To: Rob Herring
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Krzysztof Kozlowski, Andy Shevchenko, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J . Wysocki, Prashant Malani, Benson Leung, Guenter Roeck,
Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, Stephen Boyd,
dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel
Hi Rob,
Thanks for the review.
On Fri, Apr 7, 2023 at 12:15 AM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Mar 31, 2023 at 05:11:43PM +0800, Pin-yen Lin wrote:
> > ITE IT6505 can be used in systems to switch the DP traffic between
> > two downstreams, which can be USB Type-C DisplayPort alternate mode
> > lane or regular DisplayPort output ports.
> >
> > Update the binding to accommodate this usage by introducing a
> > data-lanes and a mode-switch property on endpoints.
> >
> > Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> >
> > ---
> >
> > (no changes since v12)
> >
> > Changes in v12:
> > - Fixed the schema of "data-lanes" property for it6505
> > - Reworded the description of the mode-switch property
> >
> > Changes in v11:
> > - Updated the description of the endpoints in the bindings
> > - Referenced video-interfaces.yaml instead for the endpoints binding
> > - Removed duplicated definitions from inherited schema
> >
> > Changes in v9:
> > - Fixed subject prefix again
> > - Changed the naming of the example node for it6505
> >
> > Changes in v8:
> > - Updated bindings for data-lanes property
> > - Fixed subject prefix
> >
> > Changes in v7:
> > - Fixed issues reported by dt_binding_check.
> > - Updated the schema and the example dts for data-lanes.
> > - Changed to generic naming for the example dts node.
> >
> > Changes in v6:
> > - Remove switches node and use endpoints and data-lanes property to
> > describe the connections.
> >
> > .../bindings/display/bridge/ite,it6505.yaml | 101 +++++++++++++++---
> > 1 file changed, 88 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
> > index c9a882ee6d98..348b02f26041 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
> > @@ -75,22 +75,49 @@ properties:
> > port@1:
> > $ref: /schemas/graph.yaml#/$defs/port-base
> > unevaluatedProperties: false
> > - description: Video port for DP output
> > + description:
> > + Video port for DP output. Each endpoint connects to a video output
> > + downstream, and the "data-lanes" property is used to describe the pin
> > + connections. 0, 1, 2, 3 in "data-lanes" maps to TX0, TX1, TX2, TX3,
> > + respectively.
> >
> > - properties:
> > - endpoint:
> > - $ref: /schemas/graph.yaml#/$defs/endpoint-base
> > +
> > + patternProperties:
> > + "^endpoint@[01]$":
> > + $ref: /schemas/media/video-interfaces.yaml#
> > unevaluatedProperties: false
> >
> > properties:
> > + reg: true
> > +
> > + remote-endpoint: true
>
> You don't need to list these 2.
>
> > +
> > data-lanes:
> > - minItems: 1
> > - uniqueItems: true
> > - items:
> > - - enum: [ 0, 1 ]
> > - - const: 1
> > - - const: 2
> > - - const: 3
> > + oneOf:
> > + - items:
> > + - enum: [0, 3]
>
> Only lane 1 wasn't valid? If so, mention that in the commit message.
Tha IT6505 bridge only supports "lane swapping" that reverses the
output pins, so only lane 1 or lane 2 is not valid. I'll update this
in the commit message as well.
>
> > +
> > + - items:
> > + - const: 0
> > + - const: 1
> > +
> > + - items:
> > + - const: 3
> > + - const: 2
> > +
> > + - items:
> > + - const: 0
> > + - const: 1
> > + - const: 2
> > + - const: 3
>
> Isn't this the default if 'data-lanes' is omitted.
I don't think we have a "default" in the IT6505 driver because it
currently doesn't have any use case that has all 4 pins described in
an output node. The old use case uses an extcon node to describe the
output, and a 4-pin connection makes no sense for mode switches.
However, we can drop this binding and make it the default when
'data-lanes' is omitted. This will be useful when we migrate the
extcon description to the an output port node.
>
> > +
> > + mode-switch:
> > + type: boolean
> > + description: Serves as Type-C mode switch if present.
> > +
> > + required:
> > + - reg
> > + - remote-endpoint
>
> required can be dropped.
>
> >
> > required:
> > - port@0
> > @@ -102,7 +129,6 @@ required:
> > - pwr18-supply
> > - interrupts
> > - reset-gpios
> > - - extcon
> > - ports
> >
> > additionalProperties: false
> > @@ -139,8 +165,11 @@ examples:
> > };
> >
> > port@1 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > reg = <1>;
> > - it6505_out: endpoint {
> > + it6505_out: endpoint@0 {
> > + reg = <0>;
> > remote-endpoint = <&dp_in>;
> > data-lanes = <0 1>;
> > };
> > @@ -148,3 +177,49 @@ examples:
> > };
> > };
> > };
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > + i2c {
>
> Why do we need a whole new example? Just extend the existing example
> with new properties/nodes.
I added a new example because I don't think extcon and mode switches
can co-exist. Should I merge the examples to one anyway?
>
> Rob
I'll address other comments in the next version.
Best regards,
Pin-yen
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 01/10] device property: Add remote endpoint to devcon matcher
2023-03-31 9:11 ` [PATCH v15 01/10] device property: Add remote endpoint to devcon matcher Pin-yen Lin
@ 2023-04-07 21:54 ` Stephen Boyd
0 siblings, 0 replies; 26+ messages in thread
From: Stephen Boyd @ 2023-04-07 21:54 UTC (permalink / raw)
To: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Pin-yen Lin, Prashant Malani, Rafael J . Wysocki, Rob Herring,
Robert Foss, Sakari Ailus
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, dri-devel,
linux-acpi, chrome-platform, Nícolas F . R . A . Prado,
Javier Martinez Canillas, linux-kernel
Quoting Pin-yen Lin (2023-03-31 02:11:36)
> From: Prashant Malani <pmalani@chromium.org>
>
> When searching the device graph for device matches, check the
> remote-endpoint itself for a match.
>
> Some drivers register devices for individual endpoints. This allows
> the matcher code to evaluate those for a match too, instead
> of only looking at the remote parent devices. This is required when a
> device supports two mode switches in its endpoints, so we can't simply
> register the mode switch with the parent node.
Looking at this in isolation I have no idea what a mode switch is and
how it is related to drivers/base/property.c. Can you expand on this
commit text? Maybe say two "usb typec mode switches"? And maybe include
an example graph node snippet?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-03-31 9:11 ` [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support Pin-yen Lin
2023-04-06 15:18 ` Rob Herring
@ 2023-04-12 1:38 ` Stephen Boyd
2023-04-13 9:50 ` Pin-yen Lin
1 sibling, 1 reply; 26+ messages in thread
From: Stephen Boyd @ 2023-04-12 1:38 UTC (permalink / raw)
To: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Pin-yen Lin, Prashant Malani, Rafael J . Wysocki, Rob Herring,
Robert Foss, Sakari Ailus
Cc: Xin Ji, Marek Vasut, Hsin-Yi Wang, Thomas Zimmermann,
AngeloGioacchino Del Regno, Lyude Paul, devicetree, dri-devel,
linux-acpi, chrome-platform, Nícolas F . R . A . Prado,
Javier Martinez Canillas, linux-kernel, Krzysztof Kozlowski,
Chen-Yu Tsai
Quoting Pin-yen Lin (2023-03-31 02:11:39)
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index b42553ac505c..604c7391d74f 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -12,7 +12,8 @@ maintainers:
>
> description: |
> The ANX7625 is an ultra-low power 4K Mobile HD Transmitter
> - designed for portable devices.
> + designed for portable devices. Product brief is available at
> + https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf
>
> properties:
> compatible:
> @@ -112,9 +113,40 @@ properties:
> data-lanes: true
>
> port@1:
> - $ref: /schemas/graph.yaml#/properties/port
> + $ref: /schemas/graph.yaml#/$defs/port-base
> description:
> - Video port for panel or connector.
> + Video port for panel or connector. Each endpoint connects to a video
> + output downstream, and the "data-lanes" property is used to describe
> + the pin connections. 0, 1, 2, 3 in "data-lanes" maps to SSRX1, SSTX1,
> + SSRX2, SSTX2, respectively.
> +
> + patternProperties:
> + "^endpoint@[01]$":
> + $ref: /schemas/media/video-interfaces.yaml#
> + properties:
> + reg: true
> +
> + remote-endpoint: true
> +
> + data-lanes:
> + oneOf:
> + - items:
> + - enum: [0, 1, 2, 3]
> +
> + - items:
> + - const: 0
> + - const: 1
> +
> + - items:
> + - const: 2
> + - const: 3
> +
> + mode-switch:
Is it possible to not have this property? Can we have the driver for
this anx device look at the remote-endpoint and if it sees that it is
not a drm_bridge or panel on the other end, or a DP connector, that it
should register a typec mode switch (or two depending on the number of
endpoints in port@1)? Is there any case where that doesn't hold true?
I see these possible scenarios:
1. DPI to DP bridge steering DP to one of two usb-c-connectors
In this case, endpoint@0 is connected to one usb-c-connector and
endpoint@1 is connected to another usb-c-connector. The input endpoint
is only connected to DPI. The USB endpoint is not present (although I
don't see this described in the binding either, so we would need a
port@2, entirely optional to describe USB3 input). The driver will
register two mode switches.
2. DPI to DP bridge with USB3 to one usb-c-connector
In this case, endpoint@1 doesn't exist. The SSTX1/2 and SSRX1/2 pins are
all connected to a usb-c-connector node. The input ports (0 and 2) are
connected to both DPI and USB. The device acts as both a mode-switch and
an orientation-switch. It registers both switches. I wonder if there is
any benefit to describing SBU connections or CC connections? Maybe we
don't register the orientation-switch if the SBU or CC connection isn't
described?
3. DPI to DP bridge connected to eDP panel
In this case, endpoint@1 doesn't exist. The USB endpoint is not present
(port@2). Depending on how the crosspoint should be configured, we'll
need to use data-lanes in the port@1 endpoint to describe which SSTRX
pair to use (1 or 2). Or we'll have to use the endpoint's reg property
to describe which pair to drive DP on. Presumably the default
configuration is SSRX2/SSTX2 providing 2 lanes of DP to an eDP panel.
The endpoint@0 in port@1 will be connected to a drm_panel, and the
driver will be able to detect this properly by checking for the
existence of an aux-bus node or the return value of
of_dp_aux_populate_bus().
4. DPI to DP bridge connected to DP connector
This is similar to the eDP panel scenario #3. In this case, endpoint@1
doesn't exist. The USB endpoint is not present (port@2). Same story
about port@1 and lane configuration, but we don't have an aux-bus node.
In this case, the drivers/gpu/drm/bridge/display-connector.c driver will
probe for the dp-connector node and add a drm_bridge. This anx driver
will similarly add a drm_bridge, but it needs to look at the node
connected on port@1:endpoint@0 with drm_of_get_bridge() and check if it
is a drm_bridge (DP connector) or if it is some type-c thing (connector
or orientation-switch).
I think having this mode-switch property here lets us avoid calling
drm_of_get_bridge() unconditionally in anx7625_parse_dt().
drm_of_get_bridge() will always return -EPROBE_DEFER when this is the
last drm_bridge in the chain and the other side of the endpoint is a
type-c thing (scenarios #1 and #2). Maybe we should teach
drm_of_get_bridge() that a drm_bridge might be connected to a type-c
device and have it not return -EPROBE_DEFER in that case. Or make some
new API like drm_of_get_bridge_typec() that checks if the typec
framework knows about the endpoint in question (as either a typec switch
or a connector) and returns a NULL bridge pointer. If we had that then I
think this property is not necessary.
Hopefully the usb-c-connector can always be registered with the typec
framework? I'm worried that the driver that registers the
usb-c-connector node may want to form a struct typec_port with
typec_register_port() and that will get stuck in a similar -EPROBE_DEFER
loop waiting for this mode-switch to appear. So having this property
also avoids that problem by telling typec framework to wait until this
driver can register a mode-switch.
TL;DR: Is this mode-switch property a workaround for probe defer? Can we
figure out where the mode switch is in software and not have the
property in DT? If we can it would certainly improve things because
forgetting to add the property can lead to broken behavior, and we don't
do anything like this for chains of drm_bridge devices. We just describe
the display chain and let the kernel figure out which bridge should
handle hpd, edid reading, or mode detection, etc.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-04-12 1:38 ` Stephen Boyd
@ 2023-04-13 9:50 ` Pin-yen Lin
2023-04-14 0:22 ` Stephen Boyd
0 siblings, 1 reply; 26+ messages in thread
From: Pin-yen Lin @ 2023-04-13 9:50 UTC (permalink / raw)
To: Stephen Boyd
Cc: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Prashant Malani, Rafael J . Wysocki, Rob Herring, Robert Foss,
Sakari Ailus, Xin Ji, Marek Vasut, Hsin-Yi Wang,
Thomas Zimmermann, AngeloGioacchino Del Regno, Lyude Paul,
devicetree, dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai
Hi Stephen,
On Wed, Apr 12, 2023 at 10:38 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Pin-yen Lin (2023-03-31 02:11:39)
> > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > index b42553ac505c..604c7391d74f 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > @@ -12,7 +12,8 @@ maintainers:
> >
> > description: |
> > The ANX7625 is an ultra-low power 4K Mobile HD Transmitter
> > - designed for portable devices.
> > + designed for portable devices. Product brief is available at
> > + https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf
> >
> > properties:
> > compatible:
> > @@ -112,9 +113,40 @@ properties:
> > data-lanes: true
> >
> > port@1:
> > - $ref: /schemas/graph.yaml#/properties/port
> > + $ref: /schemas/graph.yaml#/$defs/port-base
> > description:
> > - Video port for panel or connector.
> > + Video port for panel or connector. Each endpoint connects to a video
> > + output downstream, and the "data-lanes" property is used to describe
> > + the pin connections. 0, 1, 2, 3 in "data-lanes" maps to SSRX1, SSTX1,
> > + SSRX2, SSTX2, respectively.
> > +
> > + patternProperties:
> > + "^endpoint@[01]$":
> > + $ref: /schemas/media/video-interfaces.yaml#
> > + properties:
> > + reg: true
> > +
> > + remote-endpoint: true
> > +
> > + data-lanes:
> > + oneOf:
> > + - items:
> > + - enum: [0, 1, 2, 3]
> > +
> > + - items:
> > + - const: 0
> > + - const: 1
> > +
> > + - items:
> > + - const: 2
> > + - const: 3
> > +
> > + mode-switch:
>
> Is it possible to not have this property? Can we have the driver for
> this anx device look at the remote-endpoint and if it sees that it is
> not a drm_bridge or panel on the other end, or a DP connector, that it
> should register a typec mode switch (or two depending on the number of
> endpoints in port@1)? Is there any case where that doesn't hold true?
>
> I see these possible scenarios:
>
> 1. DPI to DP bridge steering DP to one of two usb-c-connectors
>
> In this case, endpoint@0 is connected to one usb-c-connector and
> endpoint@1 is connected to another usb-c-connector. The input endpoint
> is only connected to DPI. The USB endpoint is not present (although I
> don't see this described in the binding either, so we would need a
> port@2, entirely optional to describe USB3 input). The driver will
> register two mode switches.
>
> 2. DPI to DP bridge with USB3 to one usb-c-connector
>
> In this case, endpoint@1 doesn't exist. The SSTX1/2 and SSRX1/2 pins are
> all connected to a usb-c-connector node. The input ports (0 and 2) are
> connected to both DPI and USB. The device acts as both a mode-switch and
> an orientation-switch. It registers both switches. I wonder if there is
> any benefit to describing SBU connections or CC connections? Maybe we
> don't register the orientation-switch if the SBU or CC connection isn't
> described?
>
> 3. DPI to DP bridge connected to eDP panel
>
> In this case, endpoint@1 doesn't exist. The USB endpoint is not present
> (port@2). Depending on how the crosspoint should be configured, we'll
> need to use data-lanes in the port@1 endpoint to describe which SSTRX
> pair to use (1 or 2). Or we'll have to use the endpoint's reg property
> to describe which pair to drive DP on. Presumably the default
> configuration is SSRX2/SSTX2 providing 2 lanes of DP to an eDP panel.
> The endpoint@0 in port@1 will be connected to a drm_panel, and the
> driver will be able to detect this properly by checking for the
> existence of an aux-bus node or the return value of
> of_dp_aux_populate_bus().
Can we assume that the eDP panel always stays behind an `aux-bus`
node? Can't the panel be connected to the bridge directly in the
graph? Though this might not matter if we only register mode switches
when there are usb-c-connectors connected.
>
> 4. DPI to DP bridge connected to DP connector
>
> This is similar to the eDP panel scenario #3. In this case, endpoint@1
> doesn't exist. The USB endpoint is not present (port@2). Same story
> about port@1 and lane configuration, but we don't have an aux-bus node.
> In this case, the drivers/gpu/drm/bridge/display-connector.c driver will
> probe for the dp-connector node and add a drm_bridge. This anx driver
> will similarly add a drm_bridge, but it needs to look at the node
> connected on port@1:endpoint@0 with drm_of_get_bridge() and check if it
> is a drm_bridge (DP connector) or if it is some type-c thing (connector
> or orientation-switch).
>
> I think having this mode-switch property here lets us avoid calling
> drm_of_get_bridge() unconditionally in anx7625_parse_dt().
> drm_of_get_bridge() will always return -EPROBE_DEFER when this is the
> last drm_bridge in the chain and the other side of the endpoint is a
> type-c thing (scenarios #1 and #2). Maybe we should teach
> drm_of_get_bridge() that a drm_bridge might be connected to a type-c
> device and have it not return -EPROBE_DEFER in that case. Or make some
> new API like drm_of_get_bridge_typec() that checks if the typec
> framework knows about the endpoint in question (as either a typec switch
> or a connector) and returns a NULL bridge pointer. If we had that then I
> think this property is not necessary.
>
> Hopefully the usb-c-connector can always be registered with the typec
> framework? I'm worried that the driver that registers the
> usb-c-connector node may want to form a struct typec_port with
> typec_register_port() and that will get stuck in a similar -EPROBE_DEFER
> loop waiting for this mode-switch to appear. So having this property
> also avoids that problem by telling typec framework to wait until this
> driver can register a mode-switch.
>
> TL;DR: Is this mode-switch property a workaround for probe defer? Can we
> figure out where the mode switch is in software and not have the
> property in DT? If we can it would certainly improve things because
> forgetting to add the property can lead to broken behavior, and we don't
> do anything like this for chains of drm_bridge devices. We just describe
> the display chain and let the kernel figure out which bridge should
> handle hpd, edid reading, or mode detection, etc.
Actually the `mode-switch` property here is mainly because
`fwnode_typec_mux_get`[1] and `typec_mux_match`[2] only return matches
when the property is present. I am not sure what side effects would be
if I remove the ID-matching condition in `typec_mux_match`, so I added
the property here.
Is it feasible to remove the `mode-switch` property here given the
existing implementation of the Type-C framework?
[1]: https://elixir.bootlin.com/linux/latest/source/drivers/usb/typec/mux.c#L351
[2]: https://elixir.bootlin.com/linux/latest/source/drivers/usb/typec/mux.c#L290
Best regards,
Pin-yen
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-04-13 9:50 ` Pin-yen Lin
@ 2023-04-14 0:22 ` Stephen Boyd
2023-04-20 6:10 ` Stephen Boyd
0 siblings, 1 reply; 26+ messages in thread
From: Stephen Boyd @ 2023-04-14 0:22 UTC (permalink / raw)
To: Pin-yen Lin
Cc: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Prashant Malani, Rafael J . Wysocki, Rob Herring, Robert Foss,
Sakari Ailus, Xin Ji, Marek Vasut, Hsin-Yi Wang,
Thomas Zimmermann, AngeloGioacchino Del Regno, Lyude Paul,
devicetree, dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai
Quoting Pin-yen Lin (2023-04-13 02:50:44)
> Hi Stephen,
>
> On Wed, Apr 12, 2023 at 10:38 AM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Quoting Pin-yen Lin (2023-03-31 02:11:39)
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > index b42553ac505c..604c7391d74f 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > @@ -12,7 +12,8 @@ maintainers:
> > >
> > > description: |
> > > The ANX7625 is an ultra-low power 4K Mobile HD Transmitter
> > > - designed for portable devices.
> > > + designed for portable devices. Product brief is available at
> > > + https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf
> > >
> > > properties:
> > > compatible:
> > > @@ -112,9 +113,40 @@ properties:
> > > data-lanes: true
> > >
> > > port@1:
> > > - $ref: /schemas/graph.yaml#/properties/port
> > > + $ref: /schemas/graph.yaml#/$defs/port-base
> > > description:
> > > - Video port for panel or connector.
> > > + Video port for panel or connector. Each endpoint connects to a video
> > > + output downstream, and the "data-lanes" property is used to describe
> > > + the pin connections. 0, 1, 2, 3 in "data-lanes" maps to SSRX1, SSTX1,
> > > + SSRX2, SSTX2, respectively.
> > > +
> > > + patternProperties:
> > > + "^endpoint@[01]$":
> > > + $ref: /schemas/media/video-interfaces.yaml#
> > > + properties:
> > > + reg: true
> > > +
> > > + remote-endpoint: true
> > > +
> > > + data-lanes:
> > > + oneOf:
> > > + - items:
> > > + - enum: [0, 1, 2, 3]
> > > +
> > > + - items:
> > > + - const: 0
> > > + - const: 1
> > > +
> > > + - items:
> > > + - const: 2
> > > + - const: 3
> > > +
> > > + mode-switch:
> >
> > Is it possible to not have this property? Can we have the driver for
> > this anx device look at the remote-endpoint and if it sees that it is
> > not a drm_bridge or panel on the other end, or a DP connector, that it
> > should register a typec mode switch (or two depending on the number of
> > endpoints in port@1)? Is there any case where that doesn't hold true?
> >
> > I see these possible scenarios:
> >
> > 1. DPI to DP bridge steering DP to one of two usb-c-connectors
> >
> > In this case, endpoint@0 is connected to one usb-c-connector and
> > endpoint@1 is connected to another usb-c-connector. The input endpoint
> > is only connected to DPI. The USB endpoint is not present (although I
> > don't see this described in the binding either, so we would need a
> > port@2, entirely optional to describe USB3 input). The driver will
> > register two mode switches.
> >
> > 2. DPI to DP bridge with USB3 to one usb-c-connector
> >
> > In this case, endpoint@1 doesn't exist. The SSTX1/2 and SSRX1/2 pins are
> > all connected to a usb-c-connector node. The input ports (0 and 2) are
> > connected to both DPI and USB. The device acts as both a mode-switch and
> > an orientation-switch. It registers both switches. I wonder if there is
> > any benefit to describing SBU connections or CC connections? Maybe we
> > don't register the orientation-switch if the SBU or CC connection isn't
> > described?
> >
> > 3. DPI to DP bridge connected to eDP panel
> >
> > In this case, endpoint@1 doesn't exist. The USB endpoint is not present
> > (port@2). Depending on how the crosspoint should be configured, we'll
> > need to use data-lanes in the port@1 endpoint to describe which SSTRX
> > pair to use (1 or 2). Or we'll have to use the endpoint's reg property
> > to describe which pair to drive DP on. Presumably the default
> > configuration is SSRX2/SSTX2 providing 2 lanes of DP to an eDP panel.
> > The endpoint@0 in port@1 will be connected to a drm_panel, and the
> > driver will be able to detect this properly by checking for the
> > existence of an aux-bus node or the return value of
> > of_dp_aux_populate_bus().
>
> Can we assume that the eDP panel always stays behind an `aux-bus`
> node? Can't the panel be connected to the bridge directly in the
> graph? Though this might not matter if we only register mode switches
> when there are usb-c-connectors connected.
The panel is connected to the bridge in the graph. I think we should
assume the eDP panel is on an aux-bus. Maybe another scenario is a
design that has a DP to HDMI bridge wired down on the device? In which
case the output port would be connected to the HDMI bridge.
> >
> > 4. DPI to DP bridge connected to DP connector
> >
> > This is similar to the eDP panel scenario #3. In this case, endpoint@1
> > doesn't exist. The USB endpoint is not present (port@2). Same story
> > about port@1 and lane configuration, but we don't have an aux-bus node.
> > In this case, the drivers/gpu/drm/bridge/display-connector.c driver will
> > probe for the dp-connector node and add a drm_bridge. This anx driver
> > will similarly add a drm_bridge, but it needs to look at the node
> > connected on port@1:endpoint@0 with drm_of_get_bridge() and check if it
> > is a drm_bridge (DP connector) or if it is some type-c thing (connector
> > or orientation-switch).
> >
> > I think having this mode-switch property here lets us avoid calling
> > drm_of_get_bridge() unconditionally in anx7625_parse_dt().
> > drm_of_get_bridge() will always return -EPROBE_DEFER when this is the
> > last drm_bridge in the chain and the other side of the endpoint is a
> > type-c thing (scenarios #1 and #2). Maybe we should teach
> > drm_of_get_bridge() that a drm_bridge might be connected to a type-c
> > device and have it not return -EPROBE_DEFER in that case. Or make some
> > new API like drm_of_get_bridge_typec() that checks if the typec
> > framework knows about the endpoint in question (as either a typec switch
> > or a connector) and returns a NULL bridge pointer. If we had that then I
> > think this property is not necessary.
> >
> > Hopefully the usb-c-connector can always be registered with the typec
> > framework? I'm worried that the driver that registers the
> > usb-c-connector node may want to form a struct typec_port with
> > typec_register_port() and that will get stuck in a similar -EPROBE_DEFER
> > loop waiting for this mode-switch to appear. So having this property
> > also avoids that problem by telling typec framework to wait until this
> > driver can register a mode-switch.
> >
> > TL;DR: Is this mode-switch property a workaround for probe defer? Can we
> > figure out where the mode switch is in software and not have the
> > property in DT? If we can it would certainly improve things because
> > forgetting to add the property can lead to broken behavior, and we don't
> > do anything like this for chains of drm_bridge devices. We just describe
> > the display chain and let the kernel figure out which bridge should
> > handle hpd, edid reading, or mode detection, etc.
>
> Actually the `mode-switch` property here is mainly because
> `fwnode_typec_mux_get`[1] and `typec_mux_match`[2] only return matches
> when the property is present. I am not sure what side effects would be
> if I remove the ID-matching condition in `typec_mux_match`, so I added
> the property here.
>
> Is it feasible to remove the `mode-switch` property here given the
> existing implementation of the Type-C framework?
Omitting the mode-switch property would require changes to the type-c
framework.
I'm wondering if we can have this anx driver register mode switches for
however many endpoints exist in the output port all the time when the
aux-bus node doesn't exist. Then the type-c framework can walk from the
usb-c-connector to each connected node looking for a device that is both
a drm_bridge and a mode-switch. When it finds that combination, it knows
that the mode-switch has been found. This hinges on the idea that a
device that would have the mode-switch property is a drm_bridge and
would register a mode-switch with the type-c framework.
It may be a little complicated though, because we would only register
one drm_bridge for the input to this anx device. The type-c walking code
would need to look at the graph endpoint, and find the parent device to
see if it is a drm_bridge.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-04-14 0:22 ` Stephen Boyd
@ 2023-04-20 6:10 ` Stephen Boyd
2023-04-20 9:10 ` Pin-yen Lin
0 siblings, 1 reply; 26+ messages in thread
From: Stephen Boyd @ 2023-04-20 6:10 UTC (permalink / raw)
To: Pin-yen Lin
Cc: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Prashant Malani, Rafael J . Wysocki, Rob Herring, Robert Foss,
Sakari Ailus, Xin Ji, Marek Vasut, Hsin-Yi Wang,
Thomas Zimmermann, AngeloGioacchino Del Regno, Lyude Paul,
devicetree, dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai
Quoting Stephen Boyd (2023-04-13 17:22:46)
> Quoting Pin-yen Lin (2023-04-13 02:50:44)
> >
> > Actually the `mode-switch` property here is mainly because
> > `fwnode_typec_mux_get`[1] and `typec_mux_match`[2] only return matches
> > when the property is present. I am not sure what side effects would be
> > if I remove the ID-matching condition in `typec_mux_match`, so I added
> > the property here.
> >
> > Is it feasible to remove the `mode-switch` property here given the
> > existing implementation of the Type-C framework?
>
> Omitting the mode-switch property would require changes to the type-c
> framework.
>
> I'm wondering if we can have this anx driver register mode switches for
> however many endpoints exist in the output port all the time when the
> aux-bus node doesn't exist. Then the type-c framework can walk from the
> usb-c-connector to each connected node looking for a device that is both
> a drm_bridge and a mode-switch. When it finds that combination, it knows
> that the mode-switch has been found. This hinges on the idea that a
> device that would have the mode-switch property is a drm_bridge and
> would register a mode-switch with the type-c framework.
>
> It may be a little complicated though, because we would only register
> one drm_bridge for the input to this anx device. The type-c walking code
> would need to look at the graph endpoint, and find the parent device to
> see if it is a drm_bridge.
I've been thinking more about this. I think we should only have the
'mode-switch' property possible when the USB input pins (port@2) are
connected and the DPI input pins are connected (port@0). Probably you
don't have that case though?
In your case, this device should register either one or two drm_bridges
that connect to whatever downstream is actually muxing the 2 DP lanes
with the USB SS lanes onto the usb-c-connector. If that is the EC for
ChromeOS, then the EC should have a binding that accepts some number of
input ports for DP. The EC would act as a drm_bridge, or in this case
probably two bridges, and also as two type-c switches for each
drm_bridge corresponding to the usb-c-connector nodes. When DP is on the
cable, the type-c switch/mux would signal to the drm_bridge that the
display is 'connected' via DRM_BRIDGE_OP_DETECT and struct
drm_bridge_funcs::detect(). Then the drm_bridge in this anx part would
implement struct drm_bridge_funcs::atomic_enable() and configure the
crosspoint switch the right way depending on the reg property of the
output node in port@1.
Because you don't have the part that implements the orientation-switch,
you don't need to implement the code for it. I think simply adding
support in the binding for mode-switch and orientation-switch if this is
directly wired to a usb-c-connector should be sufficient. Those
properties would be at the top-level and not part of the graph binding.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-04-20 6:10 ` Stephen Boyd
@ 2023-04-20 9:10 ` Pin-yen Lin
2023-04-27 8:33 ` Pin-yen Lin
2023-04-29 4:47 ` Stephen Boyd
0 siblings, 2 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-04-20 9:10 UTC (permalink / raw)
To: Stephen Boyd
Cc: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Prashant Malani, Rafael J . Wysocki, Rob Herring, Robert Foss,
Sakari Ailus, Xin Ji, Marek Vasut, Hsin-Yi Wang,
Thomas Zimmermann, AngeloGioacchino Del Regno, Lyude Paul,
devicetree, dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai
On Thu, Apr 20, 2023 at 2:10 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Stephen Boyd (2023-04-13 17:22:46)
> > Quoting Pin-yen Lin (2023-04-13 02:50:44)
> > >
> > > Actually the `mode-switch` property here is mainly because
> > > `fwnode_typec_mux_get`[1] and `typec_mux_match`[2] only return matches
> > > when the property is present. I am not sure what side effects would be
> > > if I remove the ID-matching condition in `typec_mux_match`, so I added
> > > the property here.
> > >
> > > Is it feasible to remove the `mode-switch` property here given the
> > > existing implementation of the Type-C framework?
> >
> > Omitting the mode-switch property would require changes to the type-c
> > framework.
> >
> > I'm wondering if we can have this anx driver register mode switches for
> > however many endpoints exist in the output port all the time when the
> > aux-bus node doesn't exist. Then the type-c framework can walk from the
> > usb-c-connector to each connected node looking for a device that is both
> > a drm_bridge and a mode-switch. When it finds that combination, it knows
> > that the mode-switch has been found. This hinges on the idea that a
> > device that would have the mode-switch property is a drm_bridge and
> > would register a mode-switch with the type-c framework.
> >
> > It may be a little complicated though, because we would only register
> > one drm_bridge for the input to this anx device. The type-c walking code
> > would need to look at the graph endpoint, and find the parent device to
> > see if it is a drm_bridge.
>
> I've been thinking more about this. I think we should only have the
> 'mode-switch' property possible when the USB input pins (port@2) are
> connected and the DPI input pins are connected (port@0). Probably you
> don't have that case though?
No we don't have the use case that uses the USB input pins on anx7625.
>
> In your case, this device should register either one or two drm_bridges
> that connect to whatever downstream is actually muxing the 2 DP lanes
> with the USB SS lanes onto the usb-c-connector.
What do you mean by "muxing the 2 DP lanes with the USB SS lanes''? In
our use case, the USB data lanes from both ports are connected to a
USB hub, but the DP lanes are muxed by the crosspoint switch on
anx7625. HPD and AUX for the external display are muxed by the EC. You
can find the diagram at
https://lore.kernel.org/linux-usb/YxGzk6DNAt0aCvIY@chromium.org/
> If that is the EC for
> ChromeOS, then the EC should have a binding that accepts some number of
> input ports for DP. The EC would act as a drm_bridge, or in this case
> probably two bridges, and also as two type-c switches for each
> drm_bridge corresponding to the usb-c-connector nodes. When DP is on the
> cable, the type-c switch/mux would signal to the drm_bridge that the
> display is 'connected' via DRM_BRIDGE_OP_DETECT and struct
> drm_bridge_funcs::detect(). Then the drm_bridge in this anx part would
> implement struct drm_bridge_funcs::atomic_enable() and configure the
> crosspoint switch the right way depending on the reg property of the
> output node in port@1.
So there will be two drm bridges that act as the downstreams for
anx7625, and we find the downstream with connector_status_connected to
configure the crosspoint switch? How do we support that kind of
topology given that the drm bridge chain is currently a list? Are you
suggesting making the bridge topology to a tree, or maintaining the
two downstreams inside the anx7625 driver and not attaching them to
the bridge chain?
Also, if we still register mode switches on the two downstream
bridges, why do you prefer that over the original approach that
register switches in the anx7625 driver?
>
> Because you don't have the part that implements the orientation-switch,
> you don't need to implement the code for it. I think simply adding
> support in the binding for mode-switch and orientation-switch if this is
> directly wired to a usb-c-connector should be sufficient. Those
> properties would be at the top-level and not part of the graph binding.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-04-20 9:10 ` Pin-yen Lin
@ 2023-04-27 8:33 ` Pin-yen Lin
2023-04-29 4:47 ` Stephen Boyd
1 sibling, 0 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-04-27 8:33 UTC (permalink / raw)
To: Stephen Boyd
Cc: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Prashant Malani, Rafael J . Wysocki, Rob Herring, Robert Foss,
Sakari Ailus, Xin Ji, Marek Vasut, Hsin-Yi Wang,
Thomas Zimmermann, AngeloGioacchino Del Regno, Lyude Paul,
devicetree, dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai
On Thu, Apr 20, 2023 at 5:10 PM Pin-yen Lin <treapking@chromium.org> wrote:
>
> On Thu, Apr 20, 2023 at 2:10 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Quoting Stephen Boyd (2023-04-13 17:22:46)
> > > Quoting Pin-yen Lin (2023-04-13 02:50:44)
> > > >
> > > > Actually the `mode-switch` property here is mainly because
> > > > `fwnode_typec_mux_get`[1] and `typec_mux_match`[2] only return matches
> > > > when the property is present. I am not sure what side effects would be
> > > > if I remove the ID-matching condition in `typec_mux_match`, so I added
> > > > the property here.
> > > >
> > > > Is it feasible to remove the `mode-switch` property here given the
> > > > existing implementation of the Type-C framework?
> > >
> > > Omitting the mode-switch property would require changes to the type-c
> > > framework.
> > >
> > > I'm wondering if we can have this anx driver register mode switches for
> > > however many endpoints exist in the output port all the time when the
> > > aux-bus node doesn't exist. Then the type-c framework can walk from the
> > > usb-c-connector to each connected node looking for a device that is both
> > > a drm_bridge and a mode-switch. When it finds that combination, it knows
> > > that the mode-switch has been found. This hinges on the idea that a
> > > device that would have the mode-switch property is a drm_bridge and
> > > would register a mode-switch with the type-c framework.
I spent some time working on this approach on the Type-C side. The
issue I met is that the driver doesn't know whether a node is a
drm_bridge before the anx7625 driver probes. When there is a
"mode-switch" property in the node, the Type-C framework knows that
"here is a mode switch, but the corresponding driver hasn't registered
the typec_mux". So it returns -EPROBE_DEFER and retries later.
However, if we remove the property, the Type-C framework won't know
whether a node will be registered as a drm_bridge and register a
typec_mux.
Do you have other suggestions on this if we want to choose this approach?
Best regards,
Pin-yen
> > >
> > > It may be a little complicated though, because we would only register
> > > one drm_bridge for the input to this anx device. The type-c walking code
> > > would need to look at the graph endpoint, and find the parent device to
> > > see if it is a drm_bridge.
> >
> > I've been thinking more about this. I think we should only have the
> > 'mode-switch' property possible when the USB input pins (port@2) are
> > connected and the DPI input pins are connected (port@0). Probably you
> > don't have that case though?
>
> No we don't have the use case that uses the USB input pins on anx7625.
> >
> > In your case, this device should register either one or two drm_bridges
> > that connect to whatever downstream is actually muxing the 2 DP lanes
> > with the USB SS lanes onto the usb-c-connector.
>
> What do you mean by "muxing the 2 DP lanes with the USB SS lanes''? In
> our use case, the USB data lanes from both ports are connected to a
> USB hub, but the DP lanes are muxed by the crosspoint switch on
> anx7625. HPD and AUX for the external display are muxed by the EC. You
> can find the diagram at
> https://lore.kernel.org/linux-usb/YxGzk6DNAt0aCvIY@chromium.org/
>
> > If that is the EC for
> > ChromeOS, then the EC should have a binding that accepts some number of
> > input ports for DP. The EC would act as a drm_bridge, or in this case
> > probably two bridges, and also as two type-c switches for each
> > drm_bridge corresponding to the usb-c-connector nodes. When DP is on the
> > cable, the type-c switch/mux would signal to the drm_bridge that the
> > display is 'connected' via DRM_BRIDGE_OP_DETECT and struct
> > drm_bridge_funcs::detect(). Then the drm_bridge in this anx part would
> > implement struct drm_bridge_funcs::atomic_enable() and configure the
> > crosspoint switch the right way depending on the reg property of the
> > output node in port@1.
>
> So there will be two drm bridges that act as the downstreams for
> anx7625, and we find the downstream with connector_status_connected to
> configure the crosspoint switch? How do we support that kind of
> topology given that the drm bridge chain is currently a list? Are you
> suggesting making the bridge topology to a tree, or maintaining the
> two downstreams inside the anx7625 driver and not attaching them to
> the bridge chain?
>
> Also, if we still register mode switches on the two downstream
> bridges, why do you prefer that over the original approach that
> register switches in the anx7625 driver?
>
> >
> > Because you don't have the part that implements the orientation-switch,
> > you don't need to implement the code for it. I think simply adding
> > support in the binding for mode-switch and orientation-switch if this is
> > directly wired to a usb-c-connector should be sufficient. Those
> > properties would be at the top-level and not part of the graph binding.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-04-20 9:10 ` Pin-yen Lin
2023-04-27 8:33 ` Pin-yen Lin
@ 2023-04-29 4:47 ` Stephen Boyd
2023-05-10 3:41 ` Pin-yen Lin
1 sibling, 1 reply; 26+ messages in thread
From: Stephen Boyd @ 2023-04-29 4:47 UTC (permalink / raw)
To: Pin-yen Lin
Cc: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Prashant Malani, Rafael J . Wysocki, Rob Herring, Robert Foss,
Sakari Ailus, Xin Ji, Marek Vasut, Hsin-Yi Wang,
Thomas Zimmermann, AngeloGioacchino Del Regno, Lyude Paul,
devicetree, dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai
Quoting Pin-yen Lin (2023-04-20 02:10:46)
> On Thu, Apr 20, 2023 at 2:10 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Quoting Stephen Boyd (2023-04-13 17:22:46)
> > > Quoting Pin-yen Lin (2023-04-13 02:50:44)
> > > >
> > > > Actually the `mode-switch` property here is mainly because
> > > > `fwnode_typec_mux_get`[1] and `typec_mux_match`[2] only return matches
> > > > when the property is present. I am not sure what side effects would be
> > > > if I remove the ID-matching condition in `typec_mux_match`, so I added
> > > > the property here.
> > > >
> > > > Is it feasible to remove the `mode-switch` property here given the
> > > > existing implementation of the Type-C framework?
> > >
> > > Omitting the mode-switch property would require changes to the type-c
> > > framework.
> > >
> > > I'm wondering if we can have this anx driver register mode switches for
> > > however many endpoints exist in the output port all the time when the
> > > aux-bus node doesn't exist. Then the type-c framework can walk from the
> > > usb-c-connector to each connected node looking for a device that is both
> > > a drm_bridge and a mode-switch. When it finds that combination, it knows
> > > that the mode-switch has been found. This hinges on the idea that a
> > > device that would have the mode-switch property is a drm_bridge and
> > > would register a mode-switch with the type-c framework.
> > >
> > > It may be a little complicated though, because we would only register
> > > one drm_bridge for the input to this anx device. The type-c walking code
> > > would need to look at the graph endpoint, and find the parent device to
> > > see if it is a drm_bridge.
> >
> > I've been thinking more about this. I think we should only have the
> > 'mode-switch' property possible when the USB input pins (port@2) are
> > connected and the DPI input pins are connected (port@0). Probably you
> > don't have that case though?
>
> No we don't have the use case that uses the USB input pins on anx7625.
> >
> > In your case, this device should register either one or two drm_bridges
> > that connect to whatever downstream is actually muxing the 2 DP lanes
> > with the USB SS lanes onto the usb-c-connector.
>
> What do you mean by "muxing the 2 DP lanes with the USB SS lanes''? In
> our use case, the USB data lanes from both ports are connected to a
> USB hub, but the DP lanes are muxed by the crosspoint switch on
> anx7625. HPD and AUX for the external display are muxed by the EC. You
> can find the diagram at
> https://lore.kernel.org/linux-usb/YxGzk6DNAt0aCvIY@chromium.org/
I mean that you must have some sort of orientation switch hardware that
takes the 2 DP lanes and the 2 USB SuperSpeed "lanes" or "pairs" and
puts them all onto a usb-c-connector. The usb-c-connector node can't be
connected directly to the anx7625 in your diagram because there must be
some sort of "flipper" that does the orientation control. Otherwise the
usb-c-connector wouldn't work if the user flipped the cable. Probably
this is some TCPC or redriver controlled by the EC.
>
> > If that is the EC for
> > ChromeOS, then the EC should have a binding that accepts some number of
> > input ports for DP. The EC would act as a drm_bridge, or in this case
> > probably two bridges, and also as two type-c switches for each
> > drm_bridge corresponding to the usb-c-connector nodes. When DP is on the
> > cable, the type-c switch/mux would signal to the drm_bridge that the
> > display is 'connected' via DRM_BRIDGE_OP_DETECT and struct
> > drm_bridge_funcs::detect(). Then the drm_bridge in this anx part would
> > implement struct drm_bridge_funcs::atomic_enable() and configure the
> > crosspoint switch the right way depending on the reg property of the
> > output node in port@1.
>
> So there will be two drm bridges that act as the downstreams for
> anx7625, and we find the downstream with connector_status_connected to
> configure the crosspoint switch? How do we support that kind of
> topology given that the drm bridge chain is currently a list? Are you
> suggesting making the bridge topology to a tree, or maintaining the
> two downstreams inside the anx7625 driver and not attaching them to
> the bridge chain?
Good point. I'm suggesting to make the drm bridge chain into a tree. We
need to teach drm_bridge core about a mux, and have some logic to
navigate atomically switching from one output to another. I was talking
with dianders@ and he was suggesting to use bridge attach/detach for
this. I'm not sure that will work though because that hook is only
called when the encoder is attached to the bridge.
It may also turn out that this helps with DP multi-stream transport
(MST). As far as I can recall DP MST doesn't mesh well with drm_bridge
designs because it wants to operate on a drm_connector and
drm_bridge_connector_init() wants to make only one drm_connector for a
chain of bridges. If you squint, the anx7625 could be an MST "branch"
that only supports one drm_connector being enabled at a time. Maybe that
is what we should do here, make drm_bridge support creating more than
one drm_connector and when there is a mux in the tree it walks both
sides and runs a callback similar to struct
drm_dp_mst_topology_cbs::add_connector() to tell the encoder that
there's another possible drm_connector here.
>
> Also, if we still register mode switches on the two downstream
> bridges, why do you prefer that over the original approach that
> register switches in the anx7625 driver?
I prefer to not have a mode-switch property here for a couple reasons:
1. The binding is usb type-c specific, and in the case of the IT6505
part there is nothing that indicates this is a usb type-c piece of
hardware. The IT6505 is simply a display bridge. The anx7625 part
actually does accept usb signals though, but that isn't being used or
described here. That's where my disclaimer about mode-switch making
sense applies when the usb input is used.
2. Putting mode-switch into the graph endpoint nodes is awkward. It is
a device property, and graph nodes are not devices. Some patches in
this series have to work around this fact and special case the graph
walking logic to treat the graph itself as a place to look for the
property.
3. The mode-switch property probably isn't necessary at all. The DT
reviewers have been asking why it is needed. The EC driver that
registers the usb-c-connectors can be the mode-switch and the
orientation-switch. And in reality, it _is_ both. The DP signals and
the USB signals go to the TCPC/redriver that is controlled by the EC
and the EC is the device that's doing the mode switching to push DP and
USB through the TCPC/redriver out on the right pins of the
usb-c-connector.
I guess another way to think about it is that the DP signal coming out
of the anx7625 part is not "usb type-c" at all, unless the USB signal is
coming out on the other side of the crosspoint switch and all four lanes
are wired to some usb-c-connector or redriver. Similarly, the situation
could look like trogdor, where DP is produced by the DP PHY in the SoC
and goes through an analog mux to steer DP to one or the other TCPC
that's wired to the usb-c-connector. There isn't any driver to control
that mux, but if there was it would be a gpio controlled mux that would
be a drm_bridge, because there isn't anything type-c about this
hardware.
And finally, I can see a possibility where the IT6505 is actually wired
to two different dp-connector ports. In that situation, there is no
type-c involvement, but we would still want to expose that to userspace
as two drm_connectors where only one encoder can be attached to them. If
we did that with drm_bridge, then anyone could make these sorts of
chains with muxes and it would present a sane userspace interface.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-04-29 4:47 ` Stephen Boyd
@ 2023-05-10 3:41 ` Pin-yen Lin
2023-05-11 17:30 ` Stephen Boyd
2023-05-30 6:25 ` Jagan Teki
0 siblings, 2 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-05-10 3:41 UTC (permalink / raw)
To: Stephen Boyd
Cc: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Prashant Malani, Rafael J . Wysocki, Rob Herring, Robert Foss,
Sakari Ailus, Xin Ji, Marek Vasut, Hsin-Yi Wang,
Thomas Zimmermann, AngeloGioacchino Del Regno, Lyude Paul,
devicetree, dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai, jagan
+Jagan who worked on a similar design and initiated the thread.
Hi Stephen,
On Sat, Apr 29, 2023 at 12:47 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Pin-yen Lin (2023-04-20 02:10:46)
> > On Thu, Apr 20, 2023 at 2:10 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > >
> > > Quoting Stephen Boyd (2023-04-13 17:22:46)
> > > > Quoting Pin-yen Lin (2023-04-13 02:50:44)
> > > > >
> > > > > Actually the `mode-switch` property here is mainly because
> > > > > `fwnode_typec_mux_get`[1] and `typec_mux_match`[2] only return matches
> > > > > when the property is present. I am not sure what side effects would be
> > > > > if I remove the ID-matching condition in `typec_mux_match`, so I added
> > > > > the property here.
> > > > >
> > > > > Is it feasible to remove the `mode-switch` property here given the
> > > > > existing implementation of the Type-C framework?
> > > >
> > > > Omitting the mode-switch property would require changes to the type-c
> > > > framework.
> > > >
> > > > I'm wondering if we can have this anx driver register mode switches for
> > > > however many endpoints exist in the output port all the time when the
> > > > aux-bus node doesn't exist. Then the type-c framework can walk from the
> > > > usb-c-connector to each connected node looking for a device that is both
> > > > a drm_bridge and a mode-switch. When it finds that combination, it knows
> > > > that the mode-switch has been found. This hinges on the idea that a
> > > > device that would have the mode-switch property is a drm_bridge and
> > > > would register a mode-switch with the type-c framework.
> > > >
> > > > It may be a little complicated though, because we would only register
> > > > one drm_bridge for the input to this anx device. The type-c walking code
> > > > would need to look at the graph endpoint, and find the parent device to
> > > > see if it is a drm_bridge.
> > >
> > > I've been thinking more about this. I think we should only have the
> > > 'mode-switch' property possible when the USB input pins (port@2) are
> > > connected and the DPI input pins are connected (port@0). Probably you
> > > don't have that case though?
> >
> > No we don't have the use case that uses the USB input pins on anx7625.
> > >
> > > In your case, this device should register either one or two drm_bridges
> > > that connect to whatever downstream is actually muxing the 2 DP lanes
> > > with the USB SS lanes onto the usb-c-connector.
> >
> > What do you mean by "muxing the 2 DP lanes with the USB SS lanes''? In
> > our use case, the USB data lanes from both ports are connected to a
> > USB hub, but the DP lanes are muxed by the crosspoint switch on
> > anx7625. HPD and AUX for the external display are muxed by the EC. You
> > can find the diagram at
> > https://lore.kernel.org/linux-usb/YxGzk6DNAt0aCvIY@chromium.org/
>
> I mean that you must have some sort of orientation switch hardware that
> takes the 2 DP lanes and the 2 USB SuperSpeed "lanes" or "pairs" and
> puts them all onto a usb-c-connector. The usb-c-connector node can't be
> connected directly to the anx7625 in your diagram because there must be
> some sort of "flipper" that does the orientation control. Otherwise the
> usb-c-connector wouldn't work if the user flipped the cable. Probably
> this is some TCPC or redriver controlled by the EC.
>
> >
> > > If that is the EC for
> > > ChromeOS, then the EC should have a binding that accepts some number of
> > > input ports for DP. The EC would act as a drm_bridge, or in this case
> > > probably two bridges, and also as two type-c switches for each
> > > drm_bridge corresponding to the usb-c-connector nodes. When DP is on the
> > > cable, the type-c switch/mux would signal to the drm_bridge that the
> > > display is 'connected' via DRM_BRIDGE_OP_DETECT and struct
> > > drm_bridge_funcs::detect(). Then the drm_bridge in this anx part would
> > > implement struct drm_bridge_funcs::atomic_enable() and configure the
> > > crosspoint switch the right way depending on the reg property of the
> > > output node in port@1.
> >
> > So there will be two drm bridges that act as the downstreams for
> > anx7625, and we find the downstream with connector_status_connected to
> > configure the crosspoint switch? How do we support that kind of
> > topology given that the drm bridge chain is currently a list? Are you
> > suggesting making the bridge topology to a tree, or maintaining the
> > two downstreams inside the anx7625 driver and not attaching them to
> > the bridge chain?
>
> Good point. I'm suggesting to make the drm bridge chain into a tree. We
> need to teach drm_bridge core about a mux, and have some logic to
> navigate atomically switching from one output to another. I was talking
> with dianders@ and he was suggesting to use bridge attach/detach for
> this. I'm not sure that will work though because that hook is only
> called when the encoder is attached to the bridge.
>
> It may also turn out that this helps with DP multi-stream transport
> (MST). As far as I can recall DP MST doesn't mesh well with drm_bridge
> designs because it wants to operate on a drm_connector and
> drm_bridge_connector_init() wants to make only one drm_connector for a
> chain of bridges. If you squint, the anx7625 could be an MST "branch"
> that only supports one drm_connector being enabled at a time. Maybe that
> is what we should do here, make drm_bridge support creating more than
> one drm_connector and when there is a mux in the tree it walks both
> sides and runs a callback similar to struct
> drm_dp_mst_topology_cbs::add_connector() to tell the encoder that
> there's another possible drm_connector here.
I have been surveying the approaches to change the bridge chain in
runtime, and I found this thread[1]. Quoting from Daniel:
"... exchanging the bridge chain isn't supported, there's no locking
for that since it's assumed to be invariant over the lifetime of the
drm_device instance. The simplest way to make that happen right now is to
have 2 drm_encoder instances, one with the lvds bridge chain, the other
with the hdmi bridge chain, and select the right encoder/bridge chain
depending upon which output userspace picks.
...
I wouldn't try to make bridge chains exchangeable instead, that's
headaches - e.g. with dp mst we've also opted for a bunch of fake
drm_encoders to model that kind of switching."
I'm not sure how we register two encoders properly, though. Do we make
the encoder driver check all the downstream bridges and register two
drm_encoder when a bridge is acting as a mux?
[1]: https://www.spinics.net/lists/dri-devel/msg340511.html
>
> >
> > Also, if we still register mode switches on the two downstream
> > bridges, why do you prefer that over the original approach that
> > register switches in the anx7625 driver?
>
> I prefer to not have a mode-switch property here for a couple reasons:
>
> 1. The binding is usb type-c specific, and in the case of the IT6505
> part there is nothing that indicates this is a usb type-c piece of
> hardware. The IT6505 is simply a display bridge. The anx7625 part
> actually does accept usb signals though, but that isn't being used or
> described here. That's where my disclaimer about mode-switch making
> sense applies when the usb input is used.
>
> 2. Putting mode-switch into the graph endpoint nodes is awkward. It is
> a device property, and graph nodes are not devices. Some patches in
> this series have to work around this fact and special case the graph
> walking logic to treat the graph itself as a place to look for the
> property.
>
> 3. The mode-switch property probably isn't necessary at all. The DT
> reviewers have been asking why it is needed. The EC driver that
> registers the usb-c-connectors can be the mode-switch and the
> orientation-switch. And in reality, it _is_ both. The DP signals and
> the USB signals go to the TCPC/redriver that is controlled by the EC
> and the EC is the device that's doing the mode switching to push DP and
> USB through the TCPC/redriver out on the right pins of the
> usb-c-connector.
>
> I guess another way to think about it is that the DP signal coming out
> of the anx7625 part is not "usb type-c" at all, unless the USB signal is
> coming out on the other side of the crosspoint switch and all four lanes
> are wired to some usb-c-connector or redriver. Similarly, the situation
> could look like trogdor, where DP is produced by the DP PHY in the SoC
> and goes through an analog mux to steer DP to one or the other TCPC
> that's wired to the usb-c-connector. There isn't any driver to control
> that mux, but if there was it would be a gpio controlled mux that would
> be a drm_bridge, because there isn't anything type-c about this
> hardware.
>
> And finally, I can see a possibility where the IT6505 is actually wired
> to two different dp-connector ports. In that situation, there is no
> type-c involvement, but we would still want to expose that to userspace
> as two drm_connectors where only one encoder can be attached to them. If
> we did that with drm_bridge, then anyone could make these sorts of
> chains with muxes and it would present a sane userspace interface.
Thanks for the detailed explanation. Yes, our use case in this series
is not related to the USB-C port from the bridge's perspective. The
bridge only cares about the output changes, and it doesn't need to
know whether the downstream is a USB-C port or a DP connector.
Best regards,
Pin-yen
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-05-10 3:41 ` Pin-yen Lin
@ 2023-05-11 17:30 ` Stephen Boyd
2023-05-30 5:15 ` Pin-yen Lin
2023-05-30 6:25 ` Jagan Teki
1 sibling, 1 reply; 26+ messages in thread
From: Stephen Boyd @ 2023-05-11 17:30 UTC (permalink / raw)
To: Pin-yen Lin
Cc: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Prashant Malani, Rafael J . Wysocki, Rob Herring, Robert Foss,
Sakari Ailus, Xin Ji, Marek Vasut, Hsin-Yi Wang,
Thomas Zimmermann, AngeloGioacchino Del Regno, Lyude Paul,
devicetree, dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai, jagan
Quoting Pin-yen Lin (2023-05-09 20:41:54)
> On Sat, Apr 29, 2023 at 12:47 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Good point. I'm suggesting to make the drm bridge chain into a tree. We
> > need to teach drm_bridge core about a mux, and have some logic to
> > navigate atomically switching from one output to another. I was talking
> > with dianders@ and he was suggesting to use bridge attach/detach for
> > this. I'm not sure that will work though because that hook is only
> > called when the encoder is attached to the bridge.
> >
> > It may also turn out that this helps with DP multi-stream transport
> > (MST). As far as I can recall DP MST doesn't mesh well with drm_bridge
> > designs because it wants to operate on a drm_connector and
> > drm_bridge_connector_init() wants to make only one drm_connector for a
> > chain of bridges. If you squint, the anx7625 could be an MST "branch"
> > that only supports one drm_connector being enabled at a time. Maybe that
> > is what we should do here, make drm_bridge support creating more than
> > one drm_connector and when there is a mux in the tree it walks both
> > sides and runs a callback similar to struct
> > drm_dp_mst_topology_cbs::add_connector() to tell the encoder that
> > there's another possible drm_connector here.
>
> I have been surveying the approaches to change the bridge chain in
> runtime, and I found this thread[1]. Quoting from Daniel:
I find the lore links easier to read.
> "... exchanging the bridge chain isn't supported, there's no locking
> for that since it's assumed to be invariant over the lifetime of the
> drm_device instance. The simplest way to make that happen right now is to
> have 2 drm_encoder instances, one with the lvds bridge chain, the other
> with the hdmi bridge chain, and select the right encoder/bridge chain
> depending upon which output userspace picks.
> ...
> I wouldn't try to make bridge chains exchangeable instead, that's
> headaches - e.g. with dp mst we've also opted for a bunch of fake
> drm_encoders to model that kind of switching."
>
> I'm not sure how we register two encoders properly, though. Do we make
> the encoder driver check all the downstream bridges and register two
> drm_encoder when a bridge is acting as a mux?
I honestly don't know because I'm not a DRM expert. Maybe Jagan or DRM
bridge maintainers can add to the thread here.
I kept reading the thread[2] and I think they settled on 2 drm_encoders
because they're different display formats (LVDS vs. HDMI) and 2
drm_connector structures. But then I watched the youtube video from this
thread[3] and it seems like 1 drm_encoder is actually what should be
done because there is really only DSI at the root. There's at least
three people working on this topic of muxing displays though. Good news?
The analogy between their gpio controlled mux and this anx part with a
crosspoint switch is that the gpio is like the crosspoint switch, but
the gpio is like a virtual mux? If the gpio is asserted for them, one
display bridge is enabled (HDMI) and the other one is not (LVDS).
In this case, the type-c cables may be connected to both
usb-c-connectors and HPD may be asserted on both, but only one
drm_connector will be able to attach to the 1 drm_encoder at a time. If
we had 2 drm_encoders it would be possible to attach both encoders to
both drm_connectors at the same time, which is impossible because it's a
mux. Indicating that each connector is connected is OK when HPD is high
on both usb-c-connectors. Userspace will have to attach an encoder to
the drm_connector it wants to use, and the drm_connector will indicate
which drm_encoders are possible for it, so all the information will be
provided to userspace in this design.
I think it really comes down to implementing the tree walking logic in
the drm bridge APIs. The problem is things like
drm_bridge_get_next_bridge(), drm_bridge_get_prev_bridge(), and
drm_for_each_bridge_in_chain() which will have to operate on a tree
instead of a list. There's also drm_bridge_chain_get_first_bridge() and
drm_bridge_attach(). The good news is most of these APIs are used
sparingly.
Maybe the simplest way to handle this is to maintain a tree of bridges
and generate bridge chains when an encoder is attached to a connector in
the tree. Presumably there is only one connector possible for a leaf of
the bridge tree, and one encoder at the root of the bridge chain. From
the drm_bridge structure, you'll only be able to iterate over the bridge
chain that is currently configured. Same for the encoder, you'll only be
able to walk the currently configured bridge chain from struct
drm_encoder::bridge_chain.
This hinges on the idea that the bridge_chain is a list, not a tree, and
that it only needs to exist when an encoder is attached to a connector.
When an encoder isn't attached to a connector the bridges will be in the
tree, and probably that tree structure will be maintained in the bridge
driver itself knowing that there is one input side bridge and two output
side bridges. When the input bridge is attached, it should be able to
query the output side bridges for the connector that the encoder is
attaching to and configure the mux and hook the input bridge to the
correct output bridge.
[2] https://lore.kernel.org/all/CAPj87rO7a=NbarOyZp1pE=19Lf2aGjWu7sru-eHwGjX0fpN+-A@mail.gmail.com/
[3] https://lore.kernel.org/all/CAMty3ZAQyADGLVcB13qJdEy_BiZEKpyDfCr9QrM-ucFLFPZLcw@mail.gmail.com/
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-05-11 17:30 ` Stephen Boyd
@ 2023-05-30 5:15 ` Pin-yen Lin
0 siblings, 0 replies; 26+ messages in thread
From: Pin-yen Lin @ 2023-05-30 5:15 UTC (permalink / raw)
To: Stephen Boyd
Cc: Andrzej Hajda, Andy Shevchenko, Benson Leung, Daniel Scally,
Daniel Vetter, David Airlie, Greg Kroah-Hartman, Guenter Roeck,
Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Prashant Malani, Rafael J . Wysocki, Rob Herring, Robert Foss,
Sakari Ailus, Xin Ji, Marek Vasut, Hsin-Yi Wang,
Thomas Zimmermann, AngeloGioacchino Del Regno, Lyude Paul,
devicetree, dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai, jagan
On Fri, May 12, 2023 at 1:30 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Pin-yen Lin (2023-05-09 20:41:54)
> > On Sat, Apr 29, 2023 at 12:47 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > >
> > > Good point. I'm suggesting to make the drm bridge chain into a tree. We
> > > need to teach drm_bridge core about a mux, and have some logic to
> > > navigate atomically switching from one output to another. I was talking
> > > with dianders@ and he was suggesting to use bridge attach/detach for
> > > this. I'm not sure that will work though because that hook is only
> > > called when the encoder is attached to the bridge.
> > >
> > > It may also turn out that this helps with DP multi-stream transport
> > > (MST). As far as I can recall DP MST doesn't mesh well with drm_bridge
> > > designs because it wants to operate on a drm_connector and
> > > drm_bridge_connector_init() wants to make only one drm_connector for a
> > > chain of bridges. If you squint, the anx7625 could be an MST "branch"
> > > that only supports one drm_connector being enabled at a time. Maybe that
> > > is what we should do here, make drm_bridge support creating more than
> > > one drm_connector and when there is a mux in the tree it walks both
> > > sides and runs a callback similar to struct
> > > drm_dp_mst_topology_cbs::add_connector() to tell the encoder that
> > > there's another possible drm_connector here.
> >
> > I have been surveying the approaches to change the bridge chain in
> > runtime, and I found this thread[1]. Quoting from Daniel:
>
> I find the lore links easier to read.
>
> > "... exchanging the bridge chain isn't supported, there's no locking
> > for that since it's assumed to be invariant over the lifetime of the
> > drm_device instance. The simplest way to make that happen right now is to
> > have 2 drm_encoder instances, one with the lvds bridge chain, the other
> > with the hdmi bridge chain, and select the right encoder/bridge chain
> > depending upon which output userspace picks.
> > ...
> > I wouldn't try to make bridge chains exchangeable instead, that's
> > headaches - e.g. with dp mst we've also opted for a bunch of fake
> > drm_encoders to model that kind of switching."
> >
> > I'm not sure how we register two encoders properly, though. Do we make
> > the encoder driver check all the downstream bridges and register two
> > drm_encoder when a bridge is acting as a mux?
>
> I honestly don't know because I'm not a DRM expert. Maybe Jagan or DRM
> bridge maintainers can add to the thread here.
>
> I kept reading the thread[2] and I think they settled on 2 drm_encoders
> because they're different display formats (LVDS vs. HDMI) and 2
> drm_connector structures. But then I watched the youtube video from this
> thread[3] and it seems like 1 drm_encoder is actually what should be
> done because there is really only DSI at the root. There's at least
> three people working on this topic of muxing displays though. Good news?
>
> The analogy between their gpio controlled mux and this anx part with a
> crosspoint switch is that the gpio is like the crosspoint switch, but
> the gpio is like a virtual mux? If the gpio is asserted for them, one
> display bridge is enabled (HDMI) and the other one is not (LVDS).
>
> In this case, the type-c cables may be connected to both
> usb-c-connectors and HPD may be asserted on both, but only one
> drm_connector will be able to attach to the 1 drm_encoder at a time. If
> we had 2 drm_encoders it would be possible to attach both encoders to
> both drm_connectors at the same time, which is impossible because it's a
> mux. Indicating that each connector is connected is OK when HPD is high
> on both usb-c-connectors. Userspace will have to attach an encoder to
> the drm_connector it wants to use, and the drm_connector will indicate
> which drm_encoders are possible for it, so all the information will be
> provided to userspace in this design.
>
> I think it really comes down to implementing the tree walking logic in
> the drm bridge APIs. The problem is things like
> drm_bridge_get_next_bridge(), drm_bridge_get_prev_bridge(), and
> drm_for_each_bridge_in_chain() which will have to operate on a tree
> instead of a list. There's also drm_bridge_chain_get_first_bridge() and
> drm_bridge_attach(). The good news is most of these APIs are used
> sparingly.
>
> Maybe the simplest way to handle this is to maintain a tree of bridges
> and generate bridge chains when an encoder is attached to a connector in
> the tree. Presumably there is only one connector possible for a leaf of
> the bridge tree, and one encoder at the root of the bridge chain. From
> the drm_bridge structure, you'll only be able to iterate over the bridge
> chain that is currently configured. Same for the encoder, you'll only be
> able to walk the currently configured bridge chain from struct
> drm_encoder::bridge_chain.
If I understand correctly, encoders are attached to connectors when
the driver initializes itself. Do you mean that the chain should be
generated when a connector is connected?
If so, "attach" becomes adding a bridge to the bridge "tree", and the
chain is only determined after a connector is plugged. When HPD fires,
we can use the .detect callback to find the right connector and form
the bridge chain. The following changes would need to be made to the
existing APIs:
(1) Some of the chain-traversing helpers (e.g.,
drm_for_each_bridge_in_chain()) need to be changed to traversing the
bridge tree.
(2) Drivers that hold a pointer to the connector need to either hold a
list of possible connectors or use a helper to get the active
connector at runtime.
I don't really know if this would work out. e.g., Do we really not
need the chain when the connector is disconnected? I will appreciate
any suggestions on this.
Regards,
Pin-yen
> This hinges on the idea that the bridge_chain is a list, not a tree, and
> that it only needs to exist when an encoder is attached to a connector.
> When an encoder isn't attached to a connector the bridges will be in the
> tree, and probably that tree structure will be maintained in the bridge
> driver itself knowing that there is one input side bridge and two output
> side bridges. When the input bridge is attached, it should be able to
> query the output side bridges for the connector that the encoder is
> attaching to and configure the mux and hook the input bridge to the
> correct output bridge.
>
> [2] https://lore.kernel.org/all/CAPj87rO7a=NbarOyZp1pE=19Lf2aGjWu7sru-eHwGjX0fpN+-A@mail.gmail.com/
> [3] https://lore.kernel.org/all/CAMty3ZAQyADGLVcB13qJdEy_BiZEKpyDfCr9QrM-ucFLFPZLcw@mail.gmail.com/
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support
2023-05-10 3:41 ` Pin-yen Lin
2023-05-11 17:30 ` Stephen Boyd
@ 2023-05-30 6:25 ` Jagan Teki
1 sibling, 0 replies; 26+ messages in thread
From: Jagan Teki @ 2023-05-30 6:25 UTC (permalink / raw)
To: Pin-yen Lin
Cc: Stephen Boyd, Andrzej Hajda, Andy Shevchenko, Benson Leung,
Daniel Scally, Daniel Vetter, David Airlie, Greg Kroah-Hartman,
Guenter Roeck, Heikki Krogerus, Jernej Skrabec, Jonas Karlman,
Krzysztof Kozlowski, Laurent Pinchart, Neil Armstrong,
Prashant Malani, Rafael J . Wysocki, Rob Herring, Robert Foss,
Sakari Ailus, Xin Ji, Marek Vasut, Hsin-Yi Wang,
Thomas Zimmermann, AngeloGioacchino Del Regno, Lyude Paul,
devicetree, dri-devel, linux-acpi, chrome-platform,
Nícolas F . R . A . Prado, Javier Martinez Canillas,
linux-kernel, Krzysztof Kozlowski, Chen-Yu Tsai
On Wed, May 10, 2023 at 9:12 AM Pin-yen Lin <treapking@chromium.org> wrote:
>
> +Jagan who worked on a similar design and initiated the thread.
>
> Hi Stephen,
>
> On Sat, Apr 29, 2023 at 12:47 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Quoting Pin-yen Lin (2023-04-20 02:10:46)
> > > On Thu, Apr 20, 2023 at 2:10 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > > >
> > > > Quoting Stephen Boyd (2023-04-13 17:22:46)
> > > > > Quoting Pin-yen Lin (2023-04-13 02:50:44)
> > > > > >
> > > > > > Actually the `mode-switch` property here is mainly because
> > > > > > `fwnode_typec_mux_get`[1] and `typec_mux_match`[2] only return matches
> > > > > > when the property is present. I am not sure what side effects would be
> > > > > > if I remove the ID-matching condition in `typec_mux_match`, so I added
> > > > > > the property here.
> > > > > >
> > > > > > Is it feasible to remove the `mode-switch` property here given the
> > > > > > existing implementation of the Type-C framework?
> > > > >
> > > > > Omitting the mode-switch property would require changes to the type-c
> > > > > framework.
> > > > >
> > > > > I'm wondering if we can have this anx driver register mode switches for
> > > > > however many endpoints exist in the output port all the time when the
> > > > > aux-bus node doesn't exist. Then the type-c framework can walk from the
> > > > > usb-c-connector to each connected node looking for a device that is both
> > > > > a drm_bridge and a mode-switch. When it finds that combination, it knows
> > > > > that the mode-switch has been found. This hinges on the idea that a
> > > > > device that would have the mode-switch property is a drm_bridge and
> > > > > would register a mode-switch with the type-c framework.
> > > > >
> > > > > It may be a little complicated though, because we would only register
> > > > > one drm_bridge for the input to this anx device. The type-c walking code
> > > > > would need to look at the graph endpoint, and find the parent device to
> > > > > see if it is a drm_bridge.
> > > >
> > > > I've been thinking more about this. I think we should only have the
> > > > 'mode-switch' property possible when the USB input pins (port@2) are
> > > > connected and the DPI input pins are connected (port@0). Probably you
> > > > don't have that case though?
> > >
> > > No we don't have the use case that uses the USB input pins on anx7625.
> > > >
> > > > In your case, this device should register either one or two drm_bridges
> > > > that connect to whatever downstream is actually muxing the 2 DP lanes
> > > > with the USB SS lanes onto the usb-c-connector.
> > >
> > > What do you mean by "muxing the 2 DP lanes with the USB SS lanes''? In
> > > our use case, the USB data lanes from both ports are connected to a
> > > USB hub, but the DP lanes are muxed by the crosspoint switch on
> > > anx7625. HPD and AUX for the external display are muxed by the EC. You
> > > can find the diagram at
> > > https://lore.kernel.org/linux-usb/YxGzk6DNAt0aCvIY@chromium.org/
> >
> > I mean that you must have some sort of orientation switch hardware that
> > takes the 2 DP lanes and the 2 USB SuperSpeed "lanes" or "pairs" and
> > puts them all onto a usb-c-connector. The usb-c-connector node can't be
> > connected directly to the anx7625 in your diagram because there must be
> > some sort of "flipper" that does the orientation control. Otherwise the
> > usb-c-connector wouldn't work if the user flipped the cable. Probably
> > this is some TCPC or redriver controlled by the EC.
> >
> > >
> > > > If that is the EC for
> > > > ChromeOS, then the EC should have a binding that accepts some number of
> > > > input ports for DP. The EC would act as a drm_bridge, or in this case
> > > > probably two bridges, and also as two type-c switches for each
> > > > drm_bridge corresponding to the usb-c-connector nodes. When DP is on the
> > > > cable, the type-c switch/mux would signal to the drm_bridge that the
> > > > display is 'connected' via DRM_BRIDGE_OP_DETECT and struct
> > > > drm_bridge_funcs::detect(). Then the drm_bridge in this anx part would
> > > > implement struct drm_bridge_funcs::atomic_enable() and configure the
> > > > crosspoint switch the right way depending on the reg property of the
> > > > output node in port@1.
> > >
> > > So there will be two drm bridges that act as the downstreams for
> > > anx7625, and we find the downstream with connector_status_connected to
> > > configure the crosspoint switch? How do we support that kind of
> > > topology given that the drm bridge chain is currently a list? Are you
> > > suggesting making the bridge topology to a tree, or maintaining the
> > > two downstreams inside the anx7625 driver and not attaching them to
> > > the bridge chain?
> >
> > Good point. I'm suggesting to make the drm bridge chain into a tree. We
> > need to teach drm_bridge core about a mux, and have some logic to
> > navigate atomically switching from one output to another. I was talking
> > with dianders@ and he was suggesting to use bridge attach/detach for
> > this. I'm not sure that will work though because that hook is only
> > called when the encoder is attached to the bridge.
> >
> > It may also turn out that this helps with DP multi-stream transport
> > (MST). As far as I can recall DP MST doesn't mesh well with drm_bridge
> > designs because it wants to operate on a drm_connector and
> > drm_bridge_connector_init() wants to make only one drm_connector for a
> > chain of bridges. If you squint, the anx7625 could be an MST "branch"
> > that only supports one drm_connector being enabled at a time. Maybe that
> > is what we should do here, make drm_bridge support creating more than
> > one drm_connector and when there is a mux in the tree it walks both
> > sides and runs a callback similar to struct
> > drm_dp_mst_topology_cbs::add_connector() to tell the encoder that
> > there's another possible drm_connector here.
>
> I have been surveying the approaches to change the bridge chain in
> runtime, and I found this thread[1]. Quoting from Daniel:
> "... exchanging the bridge chain isn't supported, there's no locking
> for that since it's assumed to be invariant over the lifetime of the
> drm_device instance. The simplest way to make that happen right now is to
> have 2 drm_encoder instances, one with the lvds bridge chain, the other
> with the hdmi bridge chain, and select the right encoder/bridge chain
> depending upon which output userspace picks.
> ...
> I wouldn't try to make bridge chains exchangeable instead, that's
> headaches - e.g. with dp mst we've also opted for a bunch of fake
> drm_encoders to model that kind of switching."
>
> I'm not sure how we register two encoders properly, though. Do we make
> the encoder driver check all the downstream bridges and register two
> drm_encoder when a bridge is acting as a mux?
>
> [1]: https://www.spinics.net/lists/dri-devel/msg340511.html
>
> >
> > >
> > > Also, if we still register mode switches on the two downstream
> > > bridges, why do you prefer that over the original approach that
> > > register switches in the anx7625 driver?
> >
> > I prefer to not have a mode-switch property here for a couple reasons:
> >
> > 1. The binding is usb type-c specific, and in the case of the IT6505
> > part there is nothing that indicates this is a usb type-c piece of
> > hardware. The IT6505 is simply a display bridge. The anx7625 part
> > actually does accept usb signals though, but that isn't being used or
> > described here. That's where my disclaimer about mode-switch making
> > sense applies when the usb input is used.
> >
> > 2. Putting mode-switch into the graph endpoint nodes is awkward. It is
> > a device property, and graph nodes are not devices. Some patches in
> > this series have to work around this fact and special case the graph
> > walking logic to treat the graph itself as a place to look for the
> > property.
> >
> > 3. The mode-switch property probably isn't necessary at all. The DT
> > reviewers have been asking why it is needed. The EC driver that
> > registers the usb-c-connectors can be the mode-switch and the
> > orientation-switch. And in reality, it _is_ both. The DP signals and
> > the USB signals go to the TCPC/redriver that is controlled by the EC
> > and the EC is the device that's doing the mode switching to push DP and
> > USB through the TCPC/redriver out on the right pins of the
> > usb-c-connector.
I'm hoping that I'm replying to the right in-line comments.
The design I have tried was able to switch the runtime display, where
I initially booted the system with LVDS and once Linux comes up the
console then I can connect HDMI Out cable so-that switch bridge will
detect the HPD and Turn-Off LVDS and Turn-ON HDMI Display. If I unplug
HDMI Out cable then the switch bridge detects the change of HPD and
Turn-Off HDMI and Turn ON LVDS.
The design uses a switching bridge that acts as another gpio-interrupt
bridge that sits between the DSI controller and SN65DSI84/ADV7535
bridges. From the switch bridge, the final bridge pipeline will attach
during bridge ops attach call which is indeed uncommon with respect to
some of the mainline drivers. If the switch bridge detects the changes
in HPD then the handler will update kms and which indeed updates the
bridge chain at run-time. So may be unconventional but I don't find
any other solutions other than altering the bridge chain at run time.
Incidentally, I see more switch bridges coming on this design in the
market in order to produce more than one output.
May be it is a worthy topic to discuss further.
Thanks,
Jagan.
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2023-05-30 6:25 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-31 9:11 [PATCH v15 00/10] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 01/10] device property: Add remote endpoint to devcon matcher Pin-yen Lin
2023-04-07 21:54 ` Stephen Boyd
2023-03-31 9:11 ` [PATCH v15 02/10] platform/chrome: cros_ec_typec: Purge blocking switch devlinks Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 03/10] drm/display: Add Type-C switch helpers Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 04/10] dt-bindings: display: bridge: anx7625: Add mode-switch support Pin-yen Lin
2023-04-06 15:18 ` Rob Herring
2023-04-12 1:38 ` Stephen Boyd
2023-04-13 9:50 ` Pin-yen Lin
2023-04-14 0:22 ` Stephen Boyd
2023-04-20 6:10 ` Stephen Boyd
2023-04-20 9:10 ` Pin-yen Lin
2023-04-27 8:33 ` Pin-yen Lin
2023-04-29 4:47 ` Stephen Boyd
2023-05-10 3:41 ` Pin-yen Lin
2023-05-11 17:30 ` Stephen Boyd
2023-05-30 5:15 ` Pin-yen Lin
2023-05-30 6:25 ` Jagan Teki
2023-03-31 9:11 ` [PATCH v15 05/10] drm/bridge: anx7625: Check for Type-C during panel registration Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 06/10] drm/bridge: Remove redundant i2c_client in anx7625/it6505 Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 07/10] drm/bridge: anx7625: Register Type C mode switches Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 08/10] dt-bindings: display: bridge: it6505: Add mode-switch support Pin-yen Lin
2023-04-06 15:15 ` Rob Herring
2023-04-06 15:50 ` Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 09/10] drm/bridge: it6505: Fix Kconfig indentation Pin-yen Lin
2023-03-31 9:11 ` [PATCH v15 10/10] drm/bridge: it6505: Register Type C mode switches Pin-yen Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).