[PATCH v10 0/9] Register Type-C mode-switch in DP bridge endpoints

[PATCH v10 0/9] Register Type-C mode-switch in DP bridge endpoints

[Pin-yen Lin]

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.

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.

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 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 (7):
  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: 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      |  99 ++++++++++++-
 .../bindings/display/bridge/ite,it6505.yaml   |  93 ++++++++++--
 drivers/base/property.c                       |  15 ++
 drivers/gpu/drm/bridge/Kconfig                |  21 +--
 drivers/gpu/drm/bridge/analogix/Kconfig       |   1 +
 drivers/gpu/drm/bridge/analogix/anx7625.c     | 105 +++++++++++++-
 drivers/gpu/drm/bridge/analogix/anx7625.h     |  13 ++
 drivers/gpu/drm/bridge/ite-it6505.c           | 119 +++++++++++++++-
 drivers/gpu/drm/display/drm_dp_helper.c       | 134 ++++++++++++++++++
 drivers/platform/chrome/cros_ec_typec.c       |  10 ++
 include/drm/display/drm_dp_helper.h           |  17 +++
 11 files changed, 598 insertions(+), 29 deletions(-)

-- 
2.39.0.314.g84b9a713c41-goog

[PATCH v10 1/9] device property: Add remote endpoint to devcon matcher

[Pin-yen Lin]

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: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>

---

Changes in v10:
- Collected Reviewed-by and Tested-by tags

Changes in v6:
- New in v6

 drivers/base/property.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 2a5a37fcd998..48877af4e444 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1223,6 +1223,21 @@ static unsigned int fwnode_graph_devcon_matches(struct fwnode_handle *fwnode,
 			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);
+		if (fwnode_device_is_available(node)) {
+			ret = match(node, con_id, data);
+			if (ret) {
+				if (matches)
+					matches[count] = ret;
+				count++;
+			}
+		}
+
 		node = fwnode_graph_get_remote_port_parent(ep);
 		if (!fwnode_device_is_available(node)) {
 			fwnode_handle_put(node);
-- 
2.39.0.314.g84b9a713c41-goog

Re: [PATCH v10 1/9] device property: Add remote endpoint to devcon matcher

[Heikki Krogerus]

On Thu, Jan 12, 2023 at 12:20:56PM +0800, Pin-yen Lin wrote:
> 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: Chen-Yu Tsai <wenst@chromium.org>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Changes in v10:
> - Collected Reviewed-by and Tested-by tags
> 
> Changes in v6:
> - New in v6
> 
>  drivers/base/property.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 2a5a37fcd998..48877af4e444 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1223,6 +1223,21 @@ static unsigned int fwnode_graph_devcon_matches(struct fwnode_handle *fwnode,
>  			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);
> +		if (fwnode_device_is_available(node)) {
> +			ret = match(node, con_id, data);
> +			if (ret) {
> +				if (matches)
> +					matches[count] = ret;
> +				count++;
> +			}
> +		}
> +
>  		node = fwnode_graph_get_remote_port_parent(ep);
>  		if (!fwnode_device_is_available(node)) {
>  			fwnode_handle_put(node);
> -- 
> 2.39.0.314.g84b9a713c41-goog

-- 
heikki

Re: [PATCH v10 1/9] device property: Add remote endpoint to devcon matcher

[Sakari Ailus]

Hi Pin-yen,

On Thu, Jan 12, 2023 at 12:20:56PM +0800, Pin-yen Lin wrote:
> 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: Chen-Yu Tsai <wenst@chromium.org>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> 
> ---
> 
> Changes in v10:
> - Collected Reviewed-by and Tested-by tags
> 
> Changes in v6:
> - New in v6
> 
>  drivers/base/property.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 2a5a37fcd998..48877af4e444 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1223,6 +1223,21 @@ static unsigned int fwnode_graph_devcon_matches(struct fwnode_handle *fwnode,
>  			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);
> +		if (fwnode_device_is_available(node)) {
> +			ret = match(node, con_id, data);
> +			if (ret) {
> +				if (matches)
> +					matches[count] = ret;
> +				count++;
> +			}
> +		}

Aren't you missing fwnode_handle-put(node) here??

> +
>  		node = fwnode_graph_get_remote_port_parent(ep);
>  		if (!fwnode_device_is_available(node)) {
>  			fwnode_handle_put(node);

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v10 1/9] device property: Add remote endpoint to devcon matcher

[Prashant Malani]

HI Sakari,

On Thu, Jan 12, 2023 at 5:32 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Pin-yen,
>
> On Thu, Jan 12, 2023 at 12:20:56PM +0800, Pin-yen Lin wrote:
> > From: Prashant Malani <pmalani@chromium.org>
> > +             /*
> > +              * 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);
> > +             if (fwnode_device_is_available(node)) {
> > +                     ret = match(node, con_id, data);
> > +                     if (ret) {
> > +                             if (matches)
> > +                                     matches[count] = ret;
> > +                             count++;
> > +                     }
> > +             }
>
> Aren't you missing fwnode_handle-put(node) here??

It shouldn't be necessary. We aren't break-ing/continue-ing here,
and fwnode_handle_put(node) is called latter in the loop [1][2]

BR,

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/property.c#n1256
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/property.c#n1261

Re: [PATCH v10 1/9] device property: Add remote endpoint to devcon matcher

[Andy Shevchenko]

On Thu, Jan 12, 2023 at 02:31:45PM -0800, Prashant Malani wrote:
> On Thu, Jan 12, 2023 at 5:32 AM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> > On Thu, Jan 12, 2023 at 12:20:56PM +0800, Pin-yen Lin wrote:
> > > From: Prashant Malani <pmalani@chromium.org>

...

> > > +             /*
> > > +              * 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);
> > > +             if (fwnode_device_is_available(node)) {
> > > +                     ret = match(node, con_id, data);
> > > +                     if (ret) {
> > > +                             if (matches)
> > > +                                     matches[count] = ret;
> > > +                             count++;
> > > +                     }
> > > +             }
> >
> > Aren't you missing fwnode_handle-put(node) here??
> 
> It shouldn't be necessary. We aren't break-ing/continue-ing here,
> and fwnode_handle_put(node) is called latter in the loop [1][2]
> 
> BR,
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/property.c#n1256
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/property.c#n1261

I'm really puzzled what do you mean by all this.
Sakari is right, btw.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v10 1/9] device property: Add remote endpoint to devcon matcher

[Sakari Ailus]

Hi Prashant,

On Thu, Jan 12, 2023 at 02:31:45PM -0800, Prashant Malani wrote:
> HI Sakari,
> 
> On Thu, Jan 12, 2023 at 5:32 AM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Pin-yen,
> >
> > On Thu, Jan 12, 2023 at 12:20:56PM +0800, Pin-yen Lin wrote:
> > > From: Prashant Malani <pmalani@chromium.org>
> > > +             /*
> > > +              * 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);
> > > +             if (fwnode_device_is_available(node)) {
> > > +                     ret = match(node, con_id, data);
> > > +                     if (ret) {
> > > +                             if (matches)
> > > +                                     matches[count] = ret;
> > > +                             count++;
> > > +                     }
> > > +             }
> >
> > Aren't you missing fwnode_handle-put(node) here??
> 
> It shouldn't be necessary. We aren't break-ing/continue-ing here,
> and fwnode_handle_put(node) is called latter in the loop [1][2]

It is, but node is overwritten just below this chunk --- before
fwnode_handle_put() is called on it.

> 
> BR,
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/property.c#n1256
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/property.c#n1261

-- 
Regards,

Sakari Ailus

Re: [PATCH v10 1/9] device property: Add remote endpoint to devcon matcher

[Prashant Malani]

On Mon, Jan 16, 2023 at 5:07 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Prashant,
>
> On Thu, Jan 12, 2023 at 02:31:45PM -0800, Prashant Malani wrote:
> > HI Sakari,
> >
> > On Thu, Jan 12, 2023 at 5:32 AM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > Hi Pin-yen,
> > >
> > > On Thu, Jan 12, 2023 at 12:20:56PM +0800, Pin-yen Lin wrote:
> > > > From: Prashant Malani <pmalani@chromium.org>
> > > > +             /*
> > > > +              * 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);
> > > > +             if (fwnode_device_is_available(node)) {
> > > > +                     ret = match(node, con_id, data);
> > > > +                     if (ret) {
> > > > +                             if (matches)
> > > > +                                     matches[count] = ret;
> > > > +                             count++;
> > > > +                     }
> > > > +             }
> > >
> > > Aren't you missing fwnode_handle-put(node) here??
> >
> > It shouldn't be necessary. We aren't break-ing/continue-ing here,
> > and fwnode_handle_put(node) is called latter in the loop [1][2]
>
> It is, but node is overwritten just below this chunk --- before
> fwnode_handle_put() is called on it.

Ack. Thanks for pointing that out. My bad!

Pin-yen, please make this update when you send out a v11.

BR,

-Prashant

[PATCH v10 2/9] platform/chrome: cros_ec_typec: Purge blocking switch devlinks

[Pin-yen Lin]

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>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>

---

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 2a7ff14dc37e..302474a647cc 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -382,6 +382,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.39.0.314.g84b9a713c41-goog

Re: [PATCH v10 2/9] platform/chrome: cros_ec_typec: Purge blocking switch devlinks

[Heikki Krogerus]

On Thu, Jan 12, 2023 at 12:20:57PM +0800, Pin-yen Lin wrote:
> 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>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>

FWIW:

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> 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 2a7ff14dc37e..302474a647cc 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -382,6 +382,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.39.0.314.g84b9a713c41-goog

-- 
heikki

[PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Pin-yen Lin]

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 "data-lanes" properties are available in Device Tree.

Signed-off-by: Pin-yen Lin <treapking@chromium.org>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

---

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/drm_dp_helper.c | 134 ++++++++++++++++++++++++
 include/drm/display/drm_dp_helper.h     |  17 +++
 2 files changed, 151 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 16565a0a5da6..a2ec40a621cb 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -30,11 +30,13 @@
 #include <linux/sched.h>
 #include <linux/seq_file.h>
 #include <linux/string_helpers.h>
+#include <linux/usb/typec_mux.h>
 #include <linux/dynamic_debug.h>
 
 #include <drm/display/drm_dp_helper.h>
 #include <drm/display/drm_dp_mst_helper.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_of.h>
 #include <drm/drm_print.h>
 #include <drm/drm_vblank.h>
 #include <drm/drm_panel.h>
@@ -3891,3 +3893,135 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
 EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
 
 #endif
+
+#if IS_REACHABLE(CONFIG_TYPEC)
+static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node,
+				       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 dp_lanes[2];
+	int ret, num_lanes, port_num = -1;
+
+	num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
+	if (num_lanes <= 0) {
+		dev_err(dev, "Error on getting data lanes count from %s: %d\n",
+			node->name, num_lanes);
+		return num_lanes;
+	}
+
+	ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes);
+	if (ret) {
+		dev_err(dev, "Failed to read the data-lanes variable from %s: %d\n",
+			node->name, ret);
+		return ret;
+	}
+
+	port_num = dp_lanes[0] / 2;
+
+	port_data = &switch_desc->typec_ports[port_num];
+	port_data->data = data;
+	mux_desc.fwnode = &node->fwnode;
+	mux_desc.drvdata = port_data;
+	snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
+	mux_desc.name = name;
+	mux_desc.set = mux_set;
+
+	port_data->typec_mux = typec_mux_register(dev, &mux_desc);
+	if (IS_ERR(port_data->typec_mux)) {
+		ret = PTR_ERR(port_data->typec_mux);
+		dev_err(dev, "Mode switch register for port %d failed: %d\n",
+			port_num, ret);
+
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * 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.
+ *
+ * Currently only mode switches are implemented, and the function assumes the
+ * given @port device node has endpoints with "mode-switch" property.
+ * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1,
+ * and register it as port 1 if "data-lanes" falls in 2/3.
+ */
+int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
+				   struct drm_dp_typec_switch_desc *switch_desc,
+				   void *data, typec_mux_set_fn_t mux_set)
+{
+	struct device_node *sw;
+	int ret;
+
+	for_each_child_of_node(port, sw) {
+		if (of_property_read_bool(sw, "mode-switch"))
+			switch_desc->num_typec_switches++;
+	}
+
+	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(struct drm_dp_typec_port_data), GFP_KERNEL);
+
+	if (!switch_desc->typec_ports)
+		return -ENOMEM;
+
+	/* Register switches for each connector. */
+	for_each_child_of_node(port, sw) {
+		if (!of_property_read_bool(sw, "mode-switch"))
+			continue;
+		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:
+	of_node_put(sw);
+	drm_dp_unregister_typec_switches(switch_desc);
+	dev_err(dev, "Failed to register mode switch: %d\n", ret);
+	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)
+{
+	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);
+#else
+void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
+{
+}
+EXPORT_SYMBOL(drm_dp_register_typec_switches);
+int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
+				   struct drm_dp_typec_switch_desc *switch_desc,
+				   void *data, typec_mux_set_fn_t mux_set)
+{
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
+#endif
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index ab55453f2d2c..5a3824f13b4e 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,20 @@ 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;
+	void *data;
+	bool dp_connected;
+};
+
+struct drm_dp_typec_switch_desc {
+	int num_typec_switches;
+	struct drm_dp_typec_port_data *typec_ports;
+};
+
+void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
+int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
+				   struct drm_dp_typec_switch_desc *switch_desc,
+				   void *data, typec_mux_set_fn_t mux_set);
+
 #endif /* _DRM_DP_HELPER_H_ */
-- 
2.39.0.314.g84b9a713c41-goog

Re: [PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Dmitry Baryshkov]

On 12/01/2023 06:20, Pin-yen Lin wrote:
> 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 "data-lanes" properties are available in Device Tree.
> 
> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> ---
> 
> 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/drm_dp_helper.c | 134 ++++++++++++++++++++++++
>   include/drm/display/drm_dp_helper.h     |  17 +++
>   2 files changed, 151 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index 16565a0a5da6..a2ec40a621cb 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -30,11 +30,13 @@
>   #include <linux/sched.h>
>   #include <linux/seq_file.h>
>   #include <linux/string_helpers.h>
> +#include <linux/usb/typec_mux.h>
>   #include <linux/dynamic_debug.h>
>   
>   #include <drm/display/drm_dp_helper.h>
>   #include <drm/display/drm_dp_mst_helper.h>
>   #include <drm/drm_edid.h>
> +#include <drm/drm_of.h>
>   #include <drm/drm_print.h>
>   #include <drm/drm_vblank.h>
>   #include <drm/drm_panel.h>
> @@ -3891,3 +3893,135 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
>   EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
>   
>   #endif
> +
> +#if IS_REACHABLE(CONFIG_TYPEC)
> +static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node,
> +				       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 dp_lanes[2];
> +	int ret, num_lanes, port_num = -1;
> +
> +	num_lanes = drm_of_get_data_lanes_count(node, 0, 2);

2 looks incorrect. IIRC DP altmode can support up to 4 lanes.

> +	if (num_lanes <= 0) {
> +		dev_err(dev, "Error on getting data lanes count from %s: %d\n",
> +			node->name, num_lanes);
> +		return num_lanes;
> +	}
> +
> +	ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes);
> +	if (ret) {
> +		dev_err(dev, "Failed to read the data-lanes variable from %s: %d\n",
> +			node->name, ret);
> +		return ret;
> +	}
> +
> +	port_num = dp_lanes[0] / 2;
> +
> +	port_data = &switch_desc->typec_ports[port_num];
> +	port_data->data = data;
> +	mux_desc.fwnode = &node->fwnode;
> +	mux_desc.drvdata = port_data;
> +	snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
> +	mux_desc.name = name;
> +	mux_desc.set = mux_set;
> +
> +	port_data->typec_mux = typec_mux_register(dev, &mux_desc);
> +	if (IS_ERR(port_data->typec_mux)) {
> +		ret = PTR_ERR(port_data->typec_mux);
> +		dev_err(dev, "Mode switch register for port %d failed: %d\n",
> +			port_num, ret);
> +
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * 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.
> + *
> + * Currently only mode switches are implemented, and the function assumes the
> + * given @port device node has endpoints with "mode-switch" property.
> + * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1,
> + * and register it as port 1 if "data-lanes" falls in 2/3.
> + */
> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> +				   struct drm_dp_typec_switch_desc *switch_desc,
> +				   void *data, typec_mux_set_fn_t mux_set)
> +{
> +	struct device_node *sw;
> +	int ret;
> +
> +	for_each_child_of_node(port, sw) {
> +		if (of_property_read_bool(sw, "mode-switch"))
> +			switch_desc->num_typec_switches++;
> +	}
> +
> +	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(struct drm_dp_typec_port_data), GFP_KERNEL);
> +
> +	if (!switch_desc->typec_ports)
> +		return -ENOMEM;
> +
> +	/* Register switches for each connector. */
> +	for_each_child_of_node(port, sw) {
> +		if (!of_property_read_bool(sw, "mode-switch"))
> +			continue;
> +		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:
> +	of_node_put(sw);
> +	drm_dp_unregister_typec_switches(switch_desc);
> +	dev_err(dev, "Failed to register mode switch: %d\n", ret);
> +	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)
> +{
> +	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);
> +#else
> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
> +{
> +}
> +EXPORT_SYMBOL(drm_dp_register_typec_switches);
> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> +				   struct drm_dp_typec_switch_desc *switch_desc,
> +				   void *data, typec_mux_set_fn_t mux_set)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
> +#endif
> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> index ab55453f2d2c..5a3824f13b4e 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,20 @@ 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;
> +	void *data;
> +	bool dp_connected;
> +};
> +
> +struct drm_dp_typec_switch_desc {
> +	int num_typec_switches;
> +	struct drm_dp_typec_port_data *typec_ports;
> +};
> +
> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> +				   struct drm_dp_typec_switch_desc *switch_desc,
> +				   void *data, typec_mux_set_fn_t mux_set);
> +
>   #endif /* _DRM_DP_HELPER_H_ */

-- 
With best wishes
Dmitry

Re: [PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Pin-yen Lin]

Hi Dmitry,

Thanks for the review.

On Thu, Jan 12, 2023 at 12:40 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 12/01/2023 06:20, Pin-yen Lin wrote:
> > 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 "data-lanes" properties are available in Device Tree.
> >
> > Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> > Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >
> > ---
> >
> > 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/drm_dp_helper.c | 134 ++++++++++++++++++++++++
> >   include/drm/display/drm_dp_helper.h     |  17 +++
> >   2 files changed, 151 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> > index 16565a0a5da6..a2ec40a621cb 100644
> > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > @@ -30,11 +30,13 @@
> >   #include <linux/sched.h>
> >   #include <linux/seq_file.h>
> >   #include <linux/string_helpers.h>
> > +#include <linux/usb/typec_mux.h>
> >   #include <linux/dynamic_debug.h>
> >
> >   #include <drm/display/drm_dp_helper.h>
> >   #include <drm/display/drm_dp_mst_helper.h>
> >   #include <drm/drm_edid.h>
> > +#include <drm/drm_of.h>
> >   #include <drm/drm_print.h>
> >   #include <drm/drm_vblank.h>
> >   #include <drm/drm_panel.h>
> > @@ -3891,3 +3893,135 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
> >   EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
> >
> >   #endif
> > +
> > +#if IS_REACHABLE(CONFIG_TYPEC)
> > +static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node,
> > +                                    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 dp_lanes[2];
> > +     int ret, num_lanes, port_num = -1;
> > +
> > +     num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
>
> 2 looks incorrect. IIRC DP altmode can support up to 4 lanes.

This function is implemented for 4-lane DP bridges to switch its
outputs between 2 downstreams. So, I assume that there will only be at
most 2 lanes for each downstream. I don't think a 4-lane downstream
makes sense for mode switches unless we want to support bridges with
more than 4 lanes.
>
> > +     if (num_lanes <= 0) {
> > +             dev_err(dev, "Error on getting data lanes count from %s: %d\n",
> > +                     node->name, num_lanes);
> > +             return num_lanes;
> > +     }
> > +
> > +     ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to read the data-lanes variable from %s: %d\n",
> > +                     node->name, ret);
> > +             return ret;
> > +     }
> > +
> > +     port_num = dp_lanes[0] / 2;
> > +
> > +     port_data = &switch_desc->typec_ports[port_num];
> > +     port_data->data = data;
> > +     mux_desc.fwnode = &node->fwnode;
> > +     mux_desc.drvdata = port_data;
> > +     snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
> > +     mux_desc.name = name;
> > +     mux_desc.set = mux_set;
> > +
> > +     port_data->typec_mux = typec_mux_register(dev, &mux_desc);
> > +     if (IS_ERR(port_data->typec_mux)) {
> > +             ret = PTR_ERR(port_data->typec_mux);
> > +             dev_err(dev, "Mode switch register for port %d failed: %d\n",
> > +                     port_num, ret);
> > +
> > +             return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +/**
> > + * 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.
> > + *
> > + * Currently only mode switches are implemented, and the function assumes the
> > + * given @port device node has endpoints with "mode-switch" property.
> > + * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1,
> > + * and register it as port 1 if "data-lanes" falls in 2/3.
> > + */
> > +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> > +                                struct drm_dp_typec_switch_desc *switch_desc,
> > +                                void *data, typec_mux_set_fn_t mux_set)
> > +{
> > +     struct device_node *sw;
> > +     int ret;
> > +
> > +     for_each_child_of_node(port, sw) {
> > +             if (of_property_read_bool(sw, "mode-switch"))
> > +                     switch_desc->num_typec_switches++;
> > +     }
> > +
> > +     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(struct drm_dp_typec_port_data), GFP_KERNEL);
> > +
> > +     if (!switch_desc->typec_ports)
> > +             return -ENOMEM;
> > +
> > +     /* Register switches for each connector. */
> > +     for_each_child_of_node(port, sw) {
> > +             if (!of_property_read_bool(sw, "mode-switch"))
> > +                     continue;
> > +             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:
> > +     of_node_put(sw);
> > +     drm_dp_unregister_typec_switches(switch_desc);
> > +     dev_err(dev, "Failed to register mode switch: %d\n", ret);
> > +     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)
> > +{
> > +     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);
> > +#else
> > +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
> > +{
> > +}
> > +EXPORT_SYMBOL(drm_dp_register_typec_switches);
> > +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> > +                                struct drm_dp_typec_switch_desc *switch_desc,
> > +                                void *data, typec_mux_set_fn_t mux_set)
> > +{
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
> > +#endif
> > diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> > index ab55453f2d2c..5a3824f13b4e 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,20 @@ 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;
> > +     void *data;
> > +     bool dp_connected;
> > +};
> > +
> > +struct drm_dp_typec_switch_desc {
> > +     int num_typec_switches;
> > +     struct drm_dp_typec_port_data *typec_ports;
> > +};
> > +
> > +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
> > +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> > +                                struct drm_dp_typec_switch_desc *switch_desc,
> > +                                void *data, typec_mux_set_fn_t mux_set);
> > +
> >   #endif /* _DRM_DP_HELPER_H_ */
>
> --
> With best wishes
> Dmitry
>

Best regards,
Pin-yen

Re: [PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Dmitry Baryshkov]

On 12/01/2023 07:19, Pin-yen Lin wrote:
> Hi Dmitry,
> 
> Thanks for the review.
> 
> On Thu, Jan 12, 2023 at 12:40 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> On 12/01/2023 06:20, Pin-yen Lin wrote:
>>> 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 "data-lanes" properties are available in Device Tree.
>>>
>>> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
>>> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
>>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>
>>> ---
>>>
>>> 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/drm_dp_helper.c | 134 ++++++++++++++++++++++++
>>>    include/drm/display/drm_dp_helper.h     |  17 +++
>>>    2 files changed, 151 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
>>> index 16565a0a5da6..a2ec40a621cb 100644
>>> --- a/drivers/gpu/drm/display/drm_dp_helper.c
>>> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
>>> @@ -30,11 +30,13 @@
>>>    #include <linux/sched.h>
>>>    #include <linux/seq_file.h>
>>>    #include <linux/string_helpers.h>
>>> +#include <linux/usb/typec_mux.h>
>>>    #include <linux/dynamic_debug.h>
>>>
>>>    #include <drm/display/drm_dp_helper.h>
>>>    #include <drm/display/drm_dp_mst_helper.h>
>>>    #include <drm/drm_edid.h>
>>> +#include <drm/drm_of.h>
>>>    #include <drm/drm_print.h>
>>>    #include <drm/drm_vblank.h>
>>>    #include <drm/drm_panel.h>
>>> @@ -3891,3 +3893,135 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
>>>    EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
>>>
>>>    #endif
>>> +
>>> +#if IS_REACHABLE(CONFIG_TYPEC)
>>> +static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node,
>>> +                                    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 dp_lanes[2];
>>> +     int ret, num_lanes, port_num = -1;
>>> +
>>> +     num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
>>
>> 2 looks incorrect. IIRC DP altmode can support up to 4 lanes.
> 
> This function is implemented for 4-lane DP bridges to switch its
> outputs between 2 downstreams. So, I assume that there will only be at
> most 2 lanes for each downstream. I don't think a 4-lane downstream
> makes sense for mode switches unless we want to support bridges with
> more than 4 lanes.

Yes. However by using 4 here you'd make the helper generic and cover 
both your case and the generic case. We don't need this for the msm case 
(since the mux is handled by the PHY). But if not for the PHY, I'd have 
used such helper (with max_lanes = 4).

>>
>>> +     if (num_lanes <= 0) {
>>> +             dev_err(dev, "Error on getting data lanes count from %s: %d\n",
>>> +                     node->name, num_lanes);
>>> +             return num_lanes;
>>> +     }
>>> +
>>> +     ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes);
>>> +     if (ret) {
>>> +             dev_err(dev, "Failed to read the data-lanes variable from %s: %d\n",
>>> +                     node->name, ret);
>>> +             return ret;
>>> +     }
>>> +
>>> +     port_num = dp_lanes[0] / 2;
>>> +
>>> +     port_data = &switch_desc->typec_ports[port_num];
>>> +     port_data->data = data;
>>> +     mux_desc.fwnode = &node->fwnode;
>>> +     mux_desc.drvdata = port_data;
>>> +     snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
>>> +     mux_desc.name = name;
>>> +     mux_desc.set = mux_set;
>>> +
>>> +     port_data->typec_mux = typec_mux_register(dev, &mux_desc);
>>> +     if (IS_ERR(port_data->typec_mux)) {
>>> +             ret = PTR_ERR(port_data->typec_mux);
>>> +             dev_err(dev, "Mode switch register for port %d failed: %d\n",
>>> +                     port_num, ret);
>>> +
>>> +             return ret;
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +/**
>>> + * 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.
>>> + *
>>> + * Currently only mode switches are implemented, and the function assumes the
>>> + * given @port device node has endpoints with "mode-switch" property.
>>> + * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1,
>>> + * and register it as port 1 if "data-lanes" falls in 2/3.
>>> + */
>>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
>>> +                                struct drm_dp_typec_switch_desc *switch_desc,
>>> +                                void *data, typec_mux_set_fn_t mux_set)
>>> +{
>>> +     struct device_node *sw;
>>> +     int ret;
>>> +
>>> +     for_each_child_of_node(port, sw) {
>>> +             if (of_property_read_bool(sw, "mode-switch"))
>>> +                     switch_desc->num_typec_switches++;
>>> +     }
>>> +
>>> +     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(struct drm_dp_typec_port_data), GFP_KERNEL);
>>> +
>>> +     if (!switch_desc->typec_ports)
>>> +             return -ENOMEM;
>>> +
>>> +     /* Register switches for each connector. */
>>> +     for_each_child_of_node(port, sw) {
>>> +             if (!of_property_read_bool(sw, "mode-switch"))
>>> +                     continue;
>>> +             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:
>>> +     of_node_put(sw);
>>> +     drm_dp_unregister_typec_switches(switch_desc);
>>> +     dev_err(dev, "Failed to register mode switch: %d\n", ret);
>>> +     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)
>>> +{
>>> +     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);
>>> +#else
>>> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
>>> +{
>>> +}
>>> +EXPORT_SYMBOL(drm_dp_register_typec_switches);
>>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
>>> +                                struct drm_dp_typec_switch_desc *switch_desc,
>>> +                                void *data, typec_mux_set_fn_t mux_set)
>>> +{
>>> +     return 0;
>>> +}
>>> +EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
>>> +#endif
>>> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
>>> index ab55453f2d2c..5a3824f13b4e 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,20 @@ 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;
>>> +     void *data;
>>> +     bool dp_connected;
>>> +};
>>> +
>>> +struct drm_dp_typec_switch_desc {
>>> +     int num_typec_switches;
>>> +     struct drm_dp_typec_port_data *typec_ports;
>>> +};
>>> +
>>> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
>>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
>>> +                                struct drm_dp_typec_switch_desc *switch_desc,
>>> +                                void *data, typec_mux_set_fn_t mux_set);
>>> +
>>>    #endif /* _DRM_DP_HELPER_H_ */
>>
>> --
>> With best wishes
>> Dmitry
>>
> 
> Best regards,
> Pin-yen

-- 
With best wishes
Dmitry

Re: [PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Pin-yen Lin]

On Thu, Jan 12, 2023 at 1:24 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 12/01/2023 07:19, Pin-yen Lin wrote:
> > Hi Dmitry,
> >
> > Thanks for the review.
> >
> > On Thu, Jan 12, 2023 at 12:40 PM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> >>
> >> On 12/01/2023 06:20, Pin-yen Lin wrote:
> >>> 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 "data-lanes" properties are available in Device Tree.
> >>>
> >>> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> >>> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> >>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> >>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >>>
> >>> ---
> >>>
> >>> 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/drm_dp_helper.c | 134 ++++++++++++++++++++++++
> >>>    include/drm/display/drm_dp_helper.h     |  17 +++
> >>>    2 files changed, 151 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> >>> index 16565a0a5da6..a2ec40a621cb 100644
> >>> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> >>> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> >>> @@ -30,11 +30,13 @@
> >>>    #include <linux/sched.h>
> >>>    #include <linux/seq_file.h>
> >>>    #include <linux/string_helpers.h>
> >>> +#include <linux/usb/typec_mux.h>
> >>>    #include <linux/dynamic_debug.h>
> >>>
> >>>    #include <drm/display/drm_dp_helper.h>
> >>>    #include <drm/display/drm_dp_mst_helper.h>
> >>>    #include <drm/drm_edid.h>
> >>> +#include <drm/drm_of.h>
> >>>    #include <drm/drm_print.h>
> >>>    #include <drm/drm_vblank.h>
> >>>    #include <drm/drm_panel.h>
> >>> @@ -3891,3 +3893,135 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
> >>>    EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
> >>>
> >>>    #endif
> >>> +
> >>> +#if IS_REACHABLE(CONFIG_TYPEC)
> >>> +static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node,
> >>> +                                    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 dp_lanes[2];
> >>> +     int ret, num_lanes, port_num = -1;
> >>> +
> >>> +     num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
> >>
> >> 2 looks incorrect. IIRC DP altmode can support up to 4 lanes.
> >
> > This function is implemented for 4-lane DP bridges to switch its
> > outputs between 2 downstreams. So, I assume that there will only be at
> > most 2 lanes for each downstream. I don't think a 4-lane downstream
> > makes sense for mode switches unless we want to support bridges with
> > more than 4 lanes.
>
> Yes. However by using 4 here you'd make the helper generic and cover
> both your case and the generic case. We don't need this for the msm case
> (since the mux is handled by the PHY). But if not for the PHY, I'd have
> used such helper (with max_lanes = 4).
>
I wonder if simply using 4 here really makes it more generic here.
This function assumes the mapping between "data-lanes" and the port
number (e.g., 0/1 --> port 0) and hard-coded the way to parse the
property.

Is it better to use "reg" instead of "data-lanes" to determine the
port number? The drivers can still read the DT node to get the
"data-lanes" property if they want to do some fancy stuffs around
that.
> >>
> >>> +     if (num_lanes <= 0) {
> >>> +             dev_err(dev, "Error on getting data lanes count from %s: %d\n",
> >>> +                     node->name, num_lanes);
> >>> +             return num_lanes;
> >>> +     }
> >>> +
> >>> +     ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes);
> >>> +     if (ret) {
> >>> +             dev_err(dev, "Failed to read the data-lanes variable from %s: %d\n",
> >>> +                     node->name, ret);
> >>> +             return ret;
> >>> +     }
> >>> +
> >>> +     port_num = dp_lanes[0] / 2;
> >>> +
> >>> +     port_data = &switch_desc->typec_ports[port_num];
> >>> +     port_data->data = data;
> >>> +     mux_desc.fwnode = &node->fwnode;
> >>> +     mux_desc.drvdata = port_data;
> >>> +     snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
> >>> +     mux_desc.name = name;
> >>> +     mux_desc.set = mux_set;
> >>> +
> >>> +     port_data->typec_mux = typec_mux_register(dev, &mux_desc);
> >>> +     if (IS_ERR(port_data->typec_mux)) {
> >>> +             ret = PTR_ERR(port_data->typec_mux);
> >>> +             dev_err(dev, "Mode switch register for port %d failed: %d\n",
> >>> +                     port_num, ret);
> >>> +
> >>> +             return ret;
> >>> +     }
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +/**
> >>> + * 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.
> >>> + *
> >>> + * Currently only mode switches are implemented, and the function assumes the
> >>> + * given @port device node has endpoints with "mode-switch" property.
> >>> + * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1,
> >>> + * and register it as port 1 if "data-lanes" falls in 2/3.
> >>> + */
> >>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> >>> +                                struct drm_dp_typec_switch_desc *switch_desc,
> >>> +                                void *data, typec_mux_set_fn_t mux_set)
> >>> +{
> >>> +     struct device_node *sw;
> >>> +     int ret;
> >>> +
> >>> +     for_each_child_of_node(port, sw) {
> >>> +             if (of_property_read_bool(sw, "mode-switch"))
> >>> +                     switch_desc->num_typec_switches++;
> >>> +     }
> >>> +
> >>> +     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(struct drm_dp_typec_port_data), GFP_KERNEL);
> >>> +
> >>> +     if (!switch_desc->typec_ports)
> >>> +             return -ENOMEM;
> >>> +
> >>> +     /* Register switches for each connector. */
> >>> +     for_each_child_of_node(port, sw) {
> >>> +             if (!of_property_read_bool(sw, "mode-switch"))
> >>> +                     continue;
> >>> +             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:
> >>> +     of_node_put(sw);
> >>> +     drm_dp_unregister_typec_switches(switch_desc);
> >>> +     dev_err(dev, "Failed to register mode switch: %d\n", ret);
> >>> +     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)
> >>> +{
> >>> +     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);
> >>> +#else
> >>> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
> >>> +{
> >>> +}
> >>> +EXPORT_SYMBOL(drm_dp_register_typec_switches);
> >>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> >>> +                                struct drm_dp_typec_switch_desc *switch_desc,
> >>> +                                void *data, typec_mux_set_fn_t mux_set)
> >>> +{
> >>> +     return 0;
> >>> +}
> >>> +EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
> >>> +#endif
> >>> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> >>> index ab55453f2d2c..5a3824f13b4e 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,20 @@ 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;
> >>> +     void *data;
> >>> +     bool dp_connected;
> >>> +};
> >>> +
> >>> +struct drm_dp_typec_switch_desc {
> >>> +     int num_typec_switches;
> >>> +     struct drm_dp_typec_port_data *typec_ports;
> >>> +};
> >>> +
> >>> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
> >>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> >>> +                                struct drm_dp_typec_switch_desc *switch_desc,
> >>> +                                void *data, typec_mux_set_fn_t mux_set);
> >>> +
> >>>    #endif /* _DRM_DP_HELPER_H_ */
> >>
> >> --
> >> With best wishes
> >> Dmitry
> >>
> >
> > Best regards,
> > Pin-yen
>
> --
> With best wishes
> Dmitry
>
Best regards,
Pin-yen

Re: [PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Dmitry Baryshkov]

On 12/01/2023 07:48, Pin-yen Lin wrote:
> On Thu, Jan 12, 2023 at 1:24 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> On 12/01/2023 07:19, Pin-yen Lin wrote:
>>> Hi Dmitry,
>>>
>>> Thanks for the review.
>>>
>>> On Thu, Jan 12, 2023 at 12:40 PM Dmitry Baryshkov
>>> <dmitry.baryshkov@linaro.org> wrote:
>>>>
>>>> On 12/01/2023 06:20, Pin-yen Lin wrote:
>>>>> 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 "data-lanes" properties are available in Device Tree.
>>>>>
>>>>> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
>>>>> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
>>>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
>>>>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>>>
>>>>> ---
>>>>>
>>>>> 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/drm_dp_helper.c | 134 ++++++++++++++++++++++++
>>>>>     include/drm/display/drm_dp_helper.h     |  17 +++
>>>>>     2 files changed, 151 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
>>>>> index 16565a0a5da6..a2ec40a621cb 100644
>>>>> --- a/drivers/gpu/drm/display/drm_dp_helper.c
>>>>> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
>>>>> @@ -30,11 +30,13 @@
>>>>>     #include <linux/sched.h>
>>>>>     #include <linux/seq_file.h>
>>>>>     #include <linux/string_helpers.h>
>>>>> +#include <linux/usb/typec_mux.h>
>>>>>     #include <linux/dynamic_debug.h>
>>>>>
>>>>>     #include <drm/display/drm_dp_helper.h>
>>>>>     #include <drm/display/drm_dp_mst_helper.h>
>>>>>     #include <drm/drm_edid.h>
>>>>> +#include <drm/drm_of.h>
>>>>>     #include <drm/drm_print.h>
>>>>>     #include <drm/drm_vblank.h>
>>>>>     #include <drm/drm_panel.h>
>>>>> @@ -3891,3 +3893,135 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
>>>>>     EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
>>>>>
>>>>>     #endif
>>>>> +
>>>>> +#if IS_REACHABLE(CONFIG_TYPEC)
>>>>> +static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node,
>>>>> +                                    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 dp_lanes[2];
>>>>> +     int ret, num_lanes, port_num = -1;
>>>>> +
>>>>> +     num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
>>>>
>>>> 2 looks incorrect. IIRC DP altmode can support up to 4 lanes.
>>>
>>> This function is implemented for 4-lane DP bridges to switch its
>>> outputs between 2 downstreams. So, I assume that there will only be at
>>> most 2 lanes for each downstream. I don't think a 4-lane downstream
>>> makes sense for mode switches unless we want to support bridges with
>>> more than 4 lanes.
>>
>> Yes. However by using 4 here you'd make the helper generic and cover
>> both your case and the generic case. We don't need this for the msm case
>> (since the mux is handled by the PHY). But if not for the PHY, I'd have
>> used such helper (with max_lanes = 4).
>>
> I wonder if simply using 4 here really makes it more generic here.
> This function assumes the mapping between "data-lanes" and the port
> number (e.g., 0/1 --> port 0) and hard-coded the way to parse the
> property.
> 
> Is it better to use "reg" instead of "data-lanes" to determine the
> port number? The drivers can still read the DT node to get the
> "data-lanes" property if they want to do some fancy stuffs around
> that.

Yes, I admit, this sounds more logical.

>>>>
>>>>> +     if (num_lanes <= 0) {
>>>>> +             dev_err(dev, "Error on getting data lanes count from %s: %d\n",
>>>>> +                     node->name, num_lanes);
>>>>> +             return num_lanes;
>>>>> +     }
>>>>> +
>>>>> +     ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes);
>>>>> +     if (ret) {
>>>>> +             dev_err(dev, "Failed to read the data-lanes variable from %s: %d\n",
>>>>> +                     node->name, ret);
>>>>> +             return ret;
>>>>> +     }
>>>>> +
>>>>> +     port_num = dp_lanes[0] / 2;
>>>>> +
>>>>> +     port_data = &switch_desc->typec_ports[port_num];
>>>>> +     port_data->data = data;
>>>>> +     mux_desc.fwnode = &node->fwnode;
>>>>> +     mux_desc.drvdata = port_data;
>>>>> +     snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
>>>>> +     mux_desc.name = name;
>>>>> +     mux_desc.set = mux_set;
>>>>> +
>>>>> +     port_data->typec_mux = typec_mux_register(dev, &mux_desc);
>>>>> +     if (IS_ERR(port_data->typec_mux)) {
>>>>> +             ret = PTR_ERR(port_data->typec_mux);
>>>>> +             dev_err(dev, "Mode switch register for port %d failed: %d\n",
>>>>> +                     port_num, ret);
>>>>> +
>>>>> +             return ret;
>>>>> +     }
>>>>> +
>>>>> +     return 0;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * 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.
>>>>> + *
>>>>> + * Currently only mode switches are implemented, and the function assumes the
>>>>> + * given @port device node has endpoints with "mode-switch" property.
>>>>> + * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1,
>>>>> + * and register it as port 1 if "data-lanes" falls in 2/3.
>>>>> + */
>>>>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
>>>>> +                                struct drm_dp_typec_switch_desc *switch_desc,
>>>>> +                                void *data, typec_mux_set_fn_t mux_set)
>>>>> +{
>>>>> +     struct device_node *sw;
>>>>> +     int ret;
>>>>> +
>>>>> +     for_each_child_of_node(port, sw) {
>>>>> +             if (of_property_read_bool(sw, "mode-switch"))
>>>>> +                     switch_desc->num_typec_switches++;
>>>>> +     }
>>>>> +
>>>>> +     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(struct drm_dp_typec_port_data), GFP_KERNEL);
>>>>> +
>>>>> +     if (!switch_desc->typec_ports)
>>>>> +             return -ENOMEM;
>>>>> +
>>>>> +     /* Register switches for each connector. */
>>>>> +     for_each_child_of_node(port, sw) {
>>>>> +             if (!of_property_read_bool(sw, "mode-switch"))
>>>>> +                     continue;
>>>>> +             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:
>>>>> +     of_node_put(sw);
>>>>> +     drm_dp_unregister_typec_switches(switch_desc);
>>>>> +     dev_err(dev, "Failed to register mode switch: %d\n", ret);
>>>>> +     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)
>>>>> +{
>>>>> +     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);
>>>>> +#else
>>>>> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
>>>>> +{
>>>>> +}
>>>>> +EXPORT_SYMBOL(drm_dp_register_typec_switches);
>>>>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
>>>>> +                                struct drm_dp_typec_switch_desc *switch_desc,
>>>>> +                                void *data, typec_mux_set_fn_t mux_set)
>>>>> +{
>>>>> +     return 0;
>>>>> +}
>>>>> +EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
>>>>> +#endif
>>>>> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
>>>>> index ab55453f2d2c..5a3824f13b4e 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,20 @@ 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;
>>>>> +     void *data;
>>>>> +     bool dp_connected;
>>>>> +};
>>>>> +
>>>>> +struct drm_dp_typec_switch_desc {
>>>>> +     int num_typec_switches;
>>>>> +     struct drm_dp_typec_port_data *typec_ports;
>>>>> +};
>>>>> +
>>>>> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
>>>>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
>>>>> +                                struct drm_dp_typec_switch_desc *switch_desc,
>>>>> +                                void *data, typec_mux_set_fn_t mux_set);
>>>>> +
>>>>>     #endif /* _DRM_DP_HELPER_H_ */
>>>>
>>>> --
>>>> With best wishes
>>>> Dmitry
>>>>
>>>
>>> Best regards,
>>> Pin-yen
>>
>> --
>> With best wishes
>> Dmitry
>>
> Best regards,
> Pin-yen

-- 
With best wishes
Dmitry

Re: [PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Pin-yen Lin]

On Thu, Jan 12, 2023 at 1:50 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 12/01/2023 07:48, Pin-yen Lin wrote:
> > On Thu, Jan 12, 2023 at 1:24 PM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> >>
> >> On 12/01/2023 07:19, Pin-yen Lin wrote:
> >>> Hi Dmitry,
> >>>
> >>> Thanks for the review.
> >>>
> >>> On Thu, Jan 12, 2023 at 12:40 PM Dmitry Baryshkov
> >>> <dmitry.baryshkov@linaro.org> wrote:
> >>>>
> >>>> On 12/01/2023 06:20, Pin-yen Lin wrote:
> >>>>> 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 "data-lanes" properties are available in Device Tree.
> >>>>>
> >>>>> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> >>>>> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> >>>>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> >>>>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >>>>>
> >>>>> ---
> >>>>>
> >>>>> 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/drm_dp_helper.c | 134 ++++++++++++++++++++++++
> >>>>>     include/drm/display/drm_dp_helper.h     |  17 +++
> >>>>>     2 files changed, 151 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> >>>>> index 16565a0a5da6..a2ec40a621cb 100644
> >>>>> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> >>>>> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> >>>>> @@ -30,11 +30,13 @@
> >>>>>     #include <linux/sched.h>
> >>>>>     #include <linux/seq_file.h>
> >>>>>     #include <linux/string_helpers.h>
> >>>>> +#include <linux/usb/typec_mux.h>
> >>>>>     #include <linux/dynamic_debug.h>
> >>>>>
> >>>>>     #include <drm/display/drm_dp_helper.h>
> >>>>>     #include <drm/display/drm_dp_mst_helper.h>
> >>>>>     #include <drm/drm_edid.h>
> >>>>> +#include <drm/drm_of.h>
> >>>>>     #include <drm/drm_print.h>
> >>>>>     #include <drm/drm_vblank.h>
> >>>>>     #include <drm/drm_panel.h>
> >>>>> @@ -3891,3 +3893,135 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
> >>>>>     EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
> >>>>>
> >>>>>     #endif
> >>>>> +
> >>>>> +#if IS_REACHABLE(CONFIG_TYPEC)
> >>>>> +static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node,
> >>>>> +                                    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 dp_lanes[2];
> >>>>> +     int ret, num_lanes, port_num = -1;
> >>>>> +
> >>>>> +     num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
> >>>>
> >>>> 2 looks incorrect. IIRC DP altmode can support up to 4 lanes.
> >>>
> >>> This function is implemented for 4-lane DP bridges to switch its
> >>> outputs between 2 downstreams. So, I assume that there will only be at
> >>> most 2 lanes for each downstream. I don't think a 4-lane downstream
> >>> makes sense for mode switches unless we want to support bridges with
> >>> more than 4 lanes.
> >>
> >> Yes. However by using 4 here you'd make the helper generic and cover
> >> both your case and the generic case. We don't need this for the msm case
> >> (since the mux is handled by the PHY). But if not for the PHY, I'd have
> >> used such helper (with max_lanes = 4).
> >>
> > I wonder if simply using 4 here really makes it more generic here.
> > This function assumes the mapping between "data-lanes" and the port
> > number (e.g., 0/1 --> port 0) and hard-coded the way to parse the
> > property.
> >
> > Is it better to use "reg" instead of "data-lanes" to determine the
> > port number? The drivers can still read the DT node to get the
> > "data-lanes" property if they want to do some fancy stuffs around
> > that.
>
> Yes, I admit, this sounds more logical.
>
Thanks for the reply. I'll do that in v11.
> >>>>
> >>>>> +     if (num_lanes <= 0) {
> >>>>> +             dev_err(dev, "Error on getting data lanes count from %s: %d\n",
> >>>>> +                     node->name, num_lanes);
> >>>>> +             return num_lanes;
> >>>>> +     }
> >>>>> +
> >>>>> +     ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes);
> >>>>> +     if (ret) {
> >>>>> +             dev_err(dev, "Failed to read the data-lanes variable from %s: %d\n",
> >>>>> +                     node->name, ret);
> >>>>> +             return ret;
> >>>>> +     }
> >>>>> +
> >>>>> +     port_num = dp_lanes[0] / 2;
> >>>>> +
> >>>>> +     port_data = &switch_desc->typec_ports[port_num];
> >>>>> +     port_data->data = data;
> >>>>> +     mux_desc.fwnode = &node->fwnode;
> >>>>> +     mux_desc.drvdata = port_data;
> >>>>> +     snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
> >>>>> +     mux_desc.name = name;
> >>>>> +     mux_desc.set = mux_set;
> >>>>> +
> >>>>> +     port_data->typec_mux = typec_mux_register(dev, &mux_desc);
> >>>>> +     if (IS_ERR(port_data->typec_mux)) {
> >>>>> +             ret = PTR_ERR(port_data->typec_mux);
> >>>>> +             dev_err(dev, "Mode switch register for port %d failed: %d\n",
> >>>>> +                     port_num, ret);
> >>>>> +
> >>>>> +             return ret;
> >>>>> +     }
> >>>>> +
> >>>>> +     return 0;
> >>>>> +}
> >>>>> +
> >>>>> +/**
> >>>>> + * 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.
> >>>>> + *
> >>>>> + * Currently only mode switches are implemented, and the function assumes the
> >>>>> + * given @port device node has endpoints with "mode-switch" property.
> >>>>> + * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1,
> >>>>> + * and register it as port 1 if "data-lanes" falls in 2/3.
> >>>>> + */
> >>>>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> >>>>> +                                struct drm_dp_typec_switch_desc *switch_desc,
> >>>>> +                                void *data, typec_mux_set_fn_t mux_set)
> >>>>> +{
> >>>>> +     struct device_node *sw;
> >>>>> +     int ret;
> >>>>> +
> >>>>> +     for_each_child_of_node(port, sw) {
> >>>>> +             if (of_property_read_bool(sw, "mode-switch"))
> >>>>> +                     switch_desc->num_typec_switches++;
> >>>>> +     }
> >>>>> +
> >>>>> +     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(struct drm_dp_typec_port_data), GFP_KERNEL);
> >>>>> +
> >>>>> +     if (!switch_desc->typec_ports)
> >>>>> +             return -ENOMEM;
> >>>>> +
> >>>>> +     /* Register switches for each connector. */
> >>>>> +     for_each_child_of_node(port, sw) {
> >>>>> +             if (!of_property_read_bool(sw, "mode-switch"))
> >>>>> +                     continue;
> >>>>> +             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:
> >>>>> +     of_node_put(sw);
> >>>>> +     drm_dp_unregister_typec_switches(switch_desc);
> >>>>> +     dev_err(dev, "Failed to register mode switch: %d\n", ret);
> >>>>> +     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)
> >>>>> +{
> >>>>> +     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);
> >>>>> +#else
> >>>>> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
> >>>>> +{
> >>>>> +}
> >>>>> +EXPORT_SYMBOL(drm_dp_register_typec_switches);
> >>>>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> >>>>> +                                struct drm_dp_typec_switch_desc *switch_desc,
> >>>>> +                                void *data, typec_mux_set_fn_t mux_set)
> >>>>> +{
> >>>>> +     return 0;
> >>>>> +}
> >>>>> +EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
> >>>>> +#endif
> >>>>> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> >>>>> index ab55453f2d2c..5a3824f13b4e 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,20 @@ 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;
> >>>>> +     void *data;
> >>>>> +     bool dp_connected;
> >>>>> +};
> >>>>> +
> >>>>> +struct drm_dp_typec_switch_desc {
> >>>>> +     int num_typec_switches;
> >>>>> +     struct drm_dp_typec_port_data *typec_ports;
> >>>>> +};
> >>>>> +
> >>>>> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
> >>>>> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> >>>>> +                                struct drm_dp_typec_switch_desc *switch_desc,
> >>>>> +                                void *data, typec_mux_set_fn_t mux_set);
> >>>>> +
> >>>>>     #endif /* _DRM_DP_HELPER_H_ */
> >>>>
> >>>> --
> >>>> With best wishes
> >>>> Dmitry
> >>>>
> >>>
> >>> Best regards,
> >>> Pin-yen
> >>
> >> --
> >> With best wishes
> >> Dmitry
> >>
> > Best regards,
> > Pin-yen
>
> --
> With best wishes
> Dmitry
>
Best regards,
Pin-yen

Re: [PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Jani Nikula]

On Thu, 12 Jan 2023, Pin-yen Lin <treapking@chromium.org> wrote:
> 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 "data-lanes" properties are available in Device Tree.
>
> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> ---
>
> 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/drm_dp_helper.c | 134 ++++++++++++++++++++++++
>  include/drm/display/drm_dp_helper.h     |  17 +++
>  2 files changed, 151 insertions(+)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index 16565a0a5da6..a2ec40a621cb 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -30,11 +30,13 @@
>  #include <linux/sched.h>
>  #include <linux/seq_file.h>
>  #include <linux/string_helpers.h>
> +#include <linux/usb/typec_mux.h>
>  #include <linux/dynamic_debug.h>
>  
>  #include <drm/display/drm_dp_helper.h>
>  #include <drm/display/drm_dp_mst_helper.h>
>  #include <drm/drm_edid.h>
> +#include <drm/drm_of.h>
>  #include <drm/drm_print.h>
>  #include <drm/drm_vblank.h>
>  #include <drm/drm_panel.h>
> @@ -3891,3 +3893,135 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
>  EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
>  
>  #endif
> +
> +#if IS_REACHABLE(CONFIG_TYPEC)

I think IS_REACHABLE() is a workaround for not getting the Kconfig
dependencies right. It allows configurations that silently just don't
work, instead of warning about it at config time. It fixes a build
issue, but trades it for an end user configuration issue that you don't
get any feedback about, and is hard to figure out. It's for people who
deal with build issues, but don't need to deal with user issues.

BR,
Jani.



> +static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node,
> +				       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 dp_lanes[2];
> +	int ret, num_lanes, port_num = -1;
> +
> +	num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
> +	if (num_lanes <= 0) {
> +		dev_err(dev, "Error on getting data lanes count from %s: %d\n",
> +			node->name, num_lanes);
> +		return num_lanes;
> +	}
> +
> +	ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes);
> +	if (ret) {
> +		dev_err(dev, "Failed to read the data-lanes variable from %s: %d\n",
> +			node->name, ret);
> +		return ret;
> +	}
> +
> +	port_num = dp_lanes[0] / 2;
> +
> +	port_data = &switch_desc->typec_ports[port_num];
> +	port_data->data = data;
> +	mux_desc.fwnode = &node->fwnode;
> +	mux_desc.drvdata = port_data;
> +	snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
> +	mux_desc.name = name;
> +	mux_desc.set = mux_set;
> +
> +	port_data->typec_mux = typec_mux_register(dev, &mux_desc);
> +	if (IS_ERR(port_data->typec_mux)) {
> +		ret = PTR_ERR(port_data->typec_mux);
> +		dev_err(dev, "Mode switch register for port %d failed: %d\n",
> +			port_num, ret);
> +
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * 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.
> + *
> + * Currently only mode switches are implemented, and the function assumes the
> + * given @port device node has endpoints with "mode-switch" property.
> + * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1,
> + * and register it as port 1 if "data-lanes" falls in 2/3.
> + */
> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> +				   struct drm_dp_typec_switch_desc *switch_desc,
> +				   void *data, typec_mux_set_fn_t mux_set)
> +{
> +	struct device_node *sw;
> +	int ret;
> +
> +	for_each_child_of_node(port, sw) {
> +		if (of_property_read_bool(sw, "mode-switch"))
> +			switch_desc->num_typec_switches++;
> +	}
> +
> +	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(struct drm_dp_typec_port_data), GFP_KERNEL);
> +
> +	if (!switch_desc->typec_ports)
> +		return -ENOMEM;
> +
> +	/* Register switches for each connector. */
> +	for_each_child_of_node(port, sw) {
> +		if (!of_property_read_bool(sw, "mode-switch"))
> +			continue;
> +		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:
> +	of_node_put(sw);
> +	drm_dp_unregister_typec_switches(switch_desc);
> +	dev_err(dev, "Failed to register mode switch: %d\n", ret);
> +	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)
> +{
> +	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);
> +#else
> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
> +{
> +}
> +EXPORT_SYMBOL(drm_dp_register_typec_switches);
> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> +				   struct drm_dp_typec_switch_desc *switch_desc,
> +				   void *data, typec_mux_set_fn_t mux_set)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
> +#endif
> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> index ab55453f2d2c..5a3824f13b4e 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,20 @@ 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;
> +	void *data;
> +	bool dp_connected;
> +};
> +
> +struct drm_dp_typec_switch_desc {
> +	int num_typec_switches;
> +	struct drm_dp_typec_port_data *typec_ports;
> +};
> +
> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> +				   struct drm_dp_typec_switch_desc *switch_desc,
> +				   void *data, typec_mux_set_fn_t mux_set);
> +
>  #endif /* _DRM_DP_HELPER_H_ */

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Pin-yen Lin]

Hi Jani,

Thanks for the review.

On Thu, Jan 12, 2023 at 4:37 PM Jani Nikula <jani.nikula@intel.com> wrote:
>
> On Thu, 12 Jan 2023, Pin-yen Lin <treapking@chromium.org> wrote:
> > 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 "data-lanes" properties are available in Device Tree.
> >
> > Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> > Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >
> > ---
> >
> > 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/drm_dp_helper.c | 134 ++++++++++++++++++++++++
> >  include/drm/display/drm_dp_helper.h     |  17 +++
> >  2 files changed, 151 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> > index 16565a0a5da6..a2ec40a621cb 100644
> > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > @@ -30,11 +30,13 @@
> >  #include <linux/sched.h>
> >  #include <linux/seq_file.h>
> >  #include <linux/string_helpers.h>
> > +#include <linux/usb/typec_mux.h>
> >  #include <linux/dynamic_debug.h>
> >
> >  #include <drm/display/drm_dp_helper.h>
> >  #include <drm/display/drm_dp_mst_helper.h>
> >  #include <drm/drm_edid.h>
> > +#include <drm/drm_of.h>
> >  #include <drm/drm_print.h>
> >  #include <drm/drm_vblank.h>
> >  #include <drm/drm_panel.h>
> > @@ -3891,3 +3893,135 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
> >  EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
> >
> >  #endif
> > +
> > +#if IS_REACHABLE(CONFIG_TYPEC)
>
> I think IS_REACHABLE() is a workaround for not getting the Kconfig
> dependencies right. It allows configurations that silently just don't
> work, instead of warning about it at config time. It fixes a build
> issue, but trades it for an end user configuration issue that you don't
> get any feedback about, and is hard to figure out. It's for people who
> deal with build issues, but don't need to deal with user issues.
>
> BR,
> Jani.
>
I've added "depends on TYPEC || TYPEC=n" on the Kconfigs of its users
(i.e., anx7625 and it6505). I didn't do this on DRM_DISPLAY_DP_HELPER
because that stops all users of DRM_DISPLAY_DP_HELPER, which is a
quite generic helper file, from building TYPEC as a module.

Or, do you have any other suggestions on this? Move these functions to
a separate file?

>
>
> > +static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node,
> > +                                    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 dp_lanes[2];
> > +     int ret, num_lanes, port_num = -1;
> > +
> > +     num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
> > +     if (num_lanes <= 0) {
> > +             dev_err(dev, "Error on getting data lanes count from %s: %d\n",
> > +                     node->name, num_lanes);
> > +             return num_lanes;
> > +     }
> > +
> > +     ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to read the data-lanes variable from %s: %d\n",
> > +                     node->name, ret);
> > +             return ret;
> > +     }
> > +
> > +     port_num = dp_lanes[0] / 2;
> > +
> > +     port_data = &switch_desc->typec_ports[port_num];
> > +     port_data->data = data;
> > +     mux_desc.fwnode = &node->fwnode;
> > +     mux_desc.drvdata = port_data;
> > +     snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
> > +     mux_desc.name = name;
> > +     mux_desc.set = mux_set;
> > +
> > +     port_data->typec_mux = typec_mux_register(dev, &mux_desc);
> > +     if (IS_ERR(port_data->typec_mux)) {
> > +             ret = PTR_ERR(port_data->typec_mux);
> > +             dev_err(dev, "Mode switch register for port %d failed: %d\n",
> > +                     port_num, ret);
> > +
> > +             return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +/**
> > + * 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.
> > + *
> > + * Currently only mode switches are implemented, and the function assumes the
> > + * given @port device node has endpoints with "mode-switch" property.
> > + * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1,
> > + * and register it as port 1 if "data-lanes" falls in 2/3.
> > + */
> > +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> > +                                struct drm_dp_typec_switch_desc *switch_desc,
> > +                                void *data, typec_mux_set_fn_t mux_set)
> > +{
> > +     struct device_node *sw;
> > +     int ret;
> > +
> > +     for_each_child_of_node(port, sw) {
> > +             if (of_property_read_bool(sw, "mode-switch"))
> > +                     switch_desc->num_typec_switches++;
> > +     }
> > +
> > +     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(struct drm_dp_typec_port_data), GFP_KERNEL);
> > +
> > +     if (!switch_desc->typec_ports)
> > +             return -ENOMEM;
> > +
> > +     /* Register switches for each connector. */
> > +     for_each_child_of_node(port, sw) {
> > +             if (!of_property_read_bool(sw, "mode-switch"))
> > +                     continue;
> > +             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:
> > +     of_node_put(sw);
> > +     drm_dp_unregister_typec_switches(switch_desc);
> > +     dev_err(dev, "Failed to register mode switch: %d\n", ret);
> > +     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)
> > +{
> > +     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);
> > +#else
> > +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
> > +{
> > +}
> > +EXPORT_SYMBOL(drm_dp_register_typec_switches);
> > +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> > +                                struct drm_dp_typec_switch_desc *switch_desc,
> > +                                void *data, typec_mux_set_fn_t mux_set)
> > +{
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
> > +#endif
> > diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> > index ab55453f2d2c..5a3824f13b4e 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,20 @@ 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;
> > +     void *data;
> > +     bool dp_connected;
> > +};
> > +
> > +struct drm_dp_typec_switch_desc {
> > +     int num_typec_switches;
> > +     struct drm_dp_typec_port_data *typec_ports;
> > +};
> > +
> > +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
> > +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> > +                                struct drm_dp_typec_switch_desc *switch_desc,
> > +                                void *data, typec_mux_set_fn_t mux_set);
> > +
> >  #endif /* _DRM_DP_HELPER_H_ */
>
> --
> Jani Nikula, Intel Open Source Graphics Center

Best regards,
Pin-yen

Re: [PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Heikki Krogerus]

Hi,

On Thu, Jan 12, 2023 at 12:20:58PM +0800, Pin-yen Lin wrote:
> 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 "data-lanes" properties are available in Device Tree.

Let's not make this kind of helpers DT only, please. See below ...

> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> ---
> 
> 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/drm_dp_helper.c | 134 ++++++++++++++++++++++++
>  include/drm/display/drm_dp_helper.h     |  17 +++
>  2 files changed, 151 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index 16565a0a5da6..a2ec40a621cb 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -30,11 +30,13 @@
>  #include <linux/sched.h>
>  #include <linux/seq_file.h>
>  #include <linux/string_helpers.h>
> +#include <linux/usb/typec_mux.h>
>  #include <linux/dynamic_debug.h>
>  
>  #include <drm/display/drm_dp_helper.h>
>  #include <drm/display/drm_dp_mst_helper.h>
>  #include <drm/drm_edid.h>
> +#include <drm/drm_of.h>
>  #include <drm/drm_print.h>
>  #include <drm/drm_vblank.h>
>  #include <drm/drm_panel.h>
> @@ -3891,3 +3893,135 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
>  EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
>  
>  #endif
> +
> +#if IS_REACHABLE(CONFIG_TYPEC)

I think Jani already pointed out that that is wrong. Just move these
into a separate file and enable them silently in the Makefile when
TYPEC is enabled - so no separate Kconfig option.

> +static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node,

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 dp_lanes[2];
> +	int ret, num_lanes, port_num = -1;
> +
> +	num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
> +	if (num_lanes <= 0) {

        num_lanes = fwnode_property_read_u32_array(fwnode, "data-lanes", NULL, 0);
        if (num_lanes <= 0 || num_lanes > 2)

> +		dev_err(dev, "Error on getting data lanes count from %s: %d\n",
> +			node->name, num_lanes);
> +		return num_lanes;
> +	}
> +
> +	ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes);

        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 from %s: %d\n",
> +			node->name, ret);

			fwnode_get_name(fwnode), ret);

> +		return ret;
> +	}
> +
> +	port_num = dp_lanes[0] / 2;
> +
> +	port_data = &switch_desc->typec_ports[port_num];
> +	port_data->data = data;
> +	mux_desc.fwnode = &node->fwnode;

        mux_desc.fwnode = fwnode;

> +	mux_desc.drvdata = port_data;
> +	snprintf(name, sizeof(name), "%s-%u", node->name, port_num);

	snprintf(name, sizeof(name), "%s-%u", fwnode_get_name(fwnode), port_num);

> +	mux_desc.name = name;
> +	mux_desc.set = mux_set;
> +
> +	port_data->typec_mux = typec_mux_register(dev, &mux_desc);
> +	if (IS_ERR(port_data->typec_mux)) {
> +		ret = PTR_ERR(port_data->typec_mux);
> +		dev_err(dev, "Mode switch register for port %d failed: %d\n",
> +			port_num, ret);
> +
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * 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.
> + *
> + * Currently only mode switches are implemented, and the function assumes the
> + * given @port device node has endpoints with "mode-switch" property.
> + * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1,
> + * and register it as port 1 if "data-lanes" falls in 2/3.
> + */
> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,

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 device_node *sw;

        struct fwnode_handle *sw;

> +	int ret;
> +
> +	for_each_child_of_node(port, sw) {
> +		if (of_property_read_bool(sw, "mode-switch"))
> +			switch_desc->num_typec_switches++;
> +	}

        fwnode_for_each_child_node(port, sw)
                if (fwnode_property_present(sw, "mode-switch"))
			switch_desc->num_typec_switches++;

> +	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(struct drm_dp_typec_port_data), GFP_KERNEL);
> +
> +	if (!switch_desc->typec_ports)
> +		return -ENOMEM;
> +
> +	/* Register switches for each connector. */
> +	for_each_child_of_node(port, sw) {
> +		if (!of_property_read_bool(sw, "mode-switch"))

        fwnode_for_each_child_node(port, sw) {
                if (!fwnode_property_present(sw, "mode-switch"))

> +			continue;
> +		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:
> +	of_node_put(sw);
> +	drm_dp_unregister_typec_switches(switch_desc);
> +	dev_err(dev, "Failed to register mode switch: %d\n", ret);
> +	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)
> +{
> +	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);
> +#else
> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc)
> +{
> +}
> +EXPORT_SYMBOL(drm_dp_register_typec_switches);
> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> +				   struct drm_dp_typec_switch_desc *switch_desc,
> +				   void *data, typec_mux_set_fn_t mux_set)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_unregister_typec_switches);
> +#endif
> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> index ab55453f2d2c..5a3824f13b4e 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,20 @@ 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;
> +	void *data;
> +	bool dp_connected;
> +};
> +
> +struct drm_dp_typec_switch_desc {
> +	int num_typec_switches;
> +	struct drm_dp_typec_port_data *typec_ports;
> +};
> +
> +void drm_dp_unregister_typec_switches(struct drm_dp_typec_switch_desc *switch_desc);
> +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port,
> +				   struct drm_dp_typec_switch_desc *switch_desc,
> +				   void *data, typec_mux_set_fn_t mux_set);
> +
>  #endif /* _DRM_DP_HELPER_H_ */

The function stubs go here if they are needed.


thanks,

-- 
heikki

Re: [PATCH v10 3/9] drm/display: Add Type-C switch helpers

[Andy Shevchenko]

On Fri, Jan 13, 2023 at 11:23:44AM +0200, Heikki Krogerus wrote:
> On Thu, Jan 12, 2023 at 12:20:58PM +0800, Pin-yen Lin wrote:

...

> > +		dev_err(dev, "Failed to read the data-lanes variable from %s: %d\n",
> > +			node->name, ret);
> 
> 			fwnode_get_name(fwnode), ret);

Or even %pfwP ?

...

> > +	snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
> 
> 	snprintf(name, sizeof(name), "%s-%u", fwnode_get_name(fwnode), port_num);

Ditto.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v10 4/9] dt-bindings: display: bridge: anx7625: Add mode-switch support

[Pin-yen Lin]

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>

---

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      | 99 ++++++++++++++++++-
 1 file changed, 96 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
index 4590186c4a0b..b49a350c40e3 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,10 +113,48 @@ 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.
 
+        patternProperties:
+          "^endpoint@[01]$":
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            unevaluatedProperties: false
+
+            properties:
+              reg:
+                maxItems: 1
+
+              remote-endpoint: true
+
+              data-lanes:
+                oneOf:
+                  - items:
+                      - enum: [0, 1, 2, 3]
+
+                  - items:
+                      - const: 0
+                      - const: 1
+
+                  - items:
+                      - const: 2
+                      - const: 3
+
+                  - items:
+                      - const: 0
+                      - const: 1
+                      - const: 2
+                      - const: 3
+
+              mode-switch:
+                type: boolean
+                description: Register this node as a Type-C mode switch or not.
+
+            required:
+              - reg
+              - remote-endpoint
+
     required:
       - port@0
       - port@1
@@ -164,8 +203,12 @@ examples:
                 };
 
                 mipi2dp_bridge_out: port@1 {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
                     reg = <1>;
-                    anx7625_out: endpoint {
+                    anx7625_out: endpoint@0 {
+                        reg = <0>;
                         remote-endpoint = <&panel_in>;
                     };
                 };
@@ -186,3 +229,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.39.0.314.g84b9a713c41-goog

Re: [PATCH v10 4/9] dt-bindings: display: bridge: anx7625: Add mode-switch support

[Dmitry Baryshkov]

On 12/01/2023 06:20, 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>
> 
> ---
> 
> 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      | 99 ++++++++++++++++++-
>   1 file changed, 96 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index 4590186c4a0b..b49a350c40e3 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,10 +113,48 @@ 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.
>   
> +        patternProperties:
> +          "^endpoint@[01]$":
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            unevaluatedProperties: false
> +
> +            properties:
> +              reg:
> +                maxItems: 1

Please remove duplicates to the graph.yaml. You have several of them here.

> +
> +              remote-endpoint: true
> +
> +              data-lanes:
> +                oneOf:
> +                  - items:
> +                      - enum: [0, 1, 2, 3]
> +
> +                  - items:
> +                      - const: 0
> +                      - const: 1
> +
> +                  - items:
> +                      - const: 2
> +                      - const: 3
> +
> +                  - items:
> +                      - const: 0
> +                      - const: 1
> +                      - const: 2
> +                      - const: 3
> +
> +              mode-switch:
> +                type: boolean
> +                description: Register this node as a Type-C mode switch or not.
> +
> +            required:
> +              - reg
> +              - remote-endpoint
> +
>       required:
>         - port@0
>         - port@1
> @@ -164,8 +203,12 @@ examples:
>                   };
>   
>                   mipi2dp_bridge_out: port@1 {
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +
>                       reg = <1>;
> -                    anx7625_out: endpoint {
> +                    anx7625_out: endpoint@0 {

But why? It's perfectly fine from the graph perspective to omit the 
index if there is jus a single endpoint.

> +                        reg = <0>;
>                           remote-endpoint = <&panel_in>;
>                       };
>                   };
> @@ -186,3 +229,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>;
> +                    };
> +                };
> +            };
> +        };
> +    };

-- 
With best wishes
Dmitry

Re: [PATCH v10 4/9] dt-bindings: display: bridge: anx7625: Add mode-switch support

[Pin-yen Lin]

Hi Dmitry,

Thanks for the review.

On Thu, Jan 12, 2023 at 12:43 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 12/01/2023 06:20, 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>
> >
> > ---
> >
> > 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      | 99 ++++++++++++++++++-
> >   1 file changed, 96 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > index 4590186c4a0b..b49a350c40e3 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,10 +113,48 @@ 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.
> >
> > +        patternProperties:
> > +          "^endpoint@[01]$":
> > +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> > +            unevaluatedProperties: false
> > +
> > +            properties:
> > +              reg:
> > +                maxItems: 1
>
> Please remove duplicates to the graph.yaml. You have several of them here.

I'll fix this in v11.
>
> > +
> > +              remote-endpoint: true
> > +
> > +              data-lanes:
> > +                oneOf:
> > +                  - items:
> > +                      - enum: [0, 1, 2, 3]
> > +
> > +                  - items:
> > +                      - const: 0
> > +                      - const: 1
> > +
> > +                  - items:
> > +                      - const: 2
> > +                      - const: 3
> > +
> > +                  - items:
> > +                      - const: 0
> > +                      - const: 1
> > +                      - const: 2
> > +                      - const: 3
> > +
> > +              mode-switch:
> > +                type: boolean
> > +                description: Register this node as a Type-C mode switch or not.
> > +
> > +            required:
> > +              - reg
> > +              - remote-endpoint
> > +
> >       required:
> >         - port@0
> >         - port@1
> > @@ -164,8 +203,12 @@ examples:
> >                   };
> >
> >                   mipi2dp_bridge_out: port@1 {
> > +                    #address-cells = <1>;
> > +                    #size-cells = <0>;
> > +
> >                       reg = <1>;
> > -                    anx7625_out: endpoint {
> > +                    anx7625_out: endpoint@0 {
>
> But why? It's perfectly fine from the graph perspective to omit the
> index if there is jus a single endpoint.

This is because "reg" property is added as a required property. Do you
suggest making "reg" property optional and only add it when there are
multiple ports?
>
> > +                        reg = <0>;
> >                           remote-endpoint = <&panel_in>;
> >                       };
> >                   };
> > @@ -186,3 +229,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>;
> > +                    };
> > +                };
> > +            };
> > +        };
> > +    };
>
> --
> With best wishes
> Dmitry
>

Best regards,
Pin-yen

Re: [PATCH v10 4/9] dt-bindings: display: bridge: anx7625: Add mode-switch support

[Dmitry Baryshkov]

On Thu, 12 Jan 2023 at 07:26, Pin-yen Lin <treapking@chromium.org> wrote:
>
> Hi Dmitry,
>
> Thanks for the review.
>
> On Thu, Jan 12, 2023 at 12:43 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On 12/01/2023 06:20, 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>
> > >
> > > ---
> > >
> > > 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      | 99 ++++++++++++++++++-
> > >   1 file changed, 96 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > index 4590186c4a0b..b49a350c40e3 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,10 +113,48 @@ 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.
> > >
> > > +        patternProperties:
> > > +          "^endpoint@[01]$":
> > > +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> > > +            unevaluatedProperties: false
> > > +
> > > +            properties:
> > > +              reg:
> > > +                maxItems: 1
> >
> > Please remove duplicates to the graph.yaml. You have several of them here.
>
> I'll fix this in v11.
> >
> > > +
> > > +              remote-endpoint: true
> > > +
> > > +              data-lanes:
> > > +                oneOf:
> > > +                  - items:
> > > +                      - enum: [0, 1, 2, 3]
> > > +
> > > +                  - items:
> > > +                      - const: 0
> > > +                      - const: 1
> > > +
> > > +                  - items:
> > > +                      - const: 2
> > > +                      - const: 3
> > > +
> > > +                  - items:
> > > +                      - const: 0
> > > +                      - const: 1
> > > +                      - const: 2
> > > +                      - const: 3
> > > +
> > > +              mode-switch:
> > > +                type: boolean
> > > +                description: Register this node as a Type-C mode switch or not.
> > > +
> > > +            required:
> > > +              - reg
> > > +              - remote-endpoint
> > > +
> > >       required:
> > >         - port@0
> > >         - port@1
> > > @@ -164,8 +203,12 @@ examples:
> > >                   };
> > >
> > >                   mipi2dp_bridge_out: port@1 {
> > > +                    #address-cells = <1>;
> > > +                    #size-cells = <0>;
> > > +
> > >                       reg = <1>;
> > > -                    anx7625_out: endpoint {
> > > +                    anx7625_out: endpoint@0 {
> >
> > But why? It's perfectly fine from the graph perspective to omit the
> > index if there is jus a single endpoint.
>
> This is because "reg" property is added as a required property. Do you
> suggest making "reg" property optional and only add it when there are
> multiple ports?

Yes. I think it makes sense.

> >
> > > +                        reg = <0>;
> > >                           remote-endpoint = <&panel_in>;
> > >                       };
> > >                   };
> > > @@ -186,3 +229,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>;
> > > +                    };
> > > +                };
> > > +            };
> > > +        };
> > > +    };
> >
> > --
> > With best wishes
> > Dmitry
> >
>
> Best regards,
> Pin-yen



-- 
With best wishes
Dmitry

Re: [PATCH v10 4/9] dt-bindings: display: bridge: anx7625: Add mode-switch support

[Rob Herring]

On Wed, Jan 11, 2023 at 10:21 PM Pin-yen Lin <treapking@chromium.org> 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>
>
> ---
>
> 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      | 99 ++++++++++++++++++-
>  1 file changed, 96 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index 4590186c4a0b..b49a350c40e3 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,10 +113,48 @@ 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.
>
> +        patternProperties:
> +          "^endpoint@[01]$":
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base

You are using a property from video-interfaces.yaml, so you need to
reference that.

Needs some description of what each endpoint is. IOW, how each
endpoint corresponds to the h/w.

> +            unevaluatedProperties: false
> +
> +            properties:
> +              reg:
> +                maxItems: 1
> +
> +              remote-endpoint: true
> +
> +              data-lanes:
> +                oneOf:
> +                  - items:
> +                      - enum: [0, 1, 2, 3]
> +
> +                  - items:
> +                      - const: 0
> +                      - const: 1
> +
> +                  - items:
> +                      - const: 2
> +                      - const: 3
> +
> +                  - items:
> +                      - const: 0
> +                      - const: 1
> +                      - const: 2
> +                      - const: 3
> +
> +              mode-switch:
> +                type: boolean
> +                description: Register this node as a Type-C mode switch or not.
> +
> +            required:
> +              - reg
> +              - remote-endpoint
> +
>      required:
>        - port@0
>        - port@1
> @@ -164,8 +203,12 @@ examples:
>                  };
>
>                  mipi2dp_bridge_out: port@1 {
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +
>                      reg = <1>;
> -                    anx7625_out: endpoint {
> +                    anx7625_out: endpoint@0 {
> +                        reg = <0>;
>                          remote-endpoint = <&panel_in>;
>                      };
>                  };
> @@ -186,3 +229,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.39.0.314.g84b9a713c41-goog
>

[PATCH v10 5/9] drm/bridge: anx7625: Check for Type-C during panel registration

[Pin-yen Lin]

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: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>

---

Changes in v10:
- Collected Reviewed-by and Tested-by tags

Changes in v6:
- New in v6

 drivers/gpu/drm/bridge/analogix/anx7625.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index b375887e655d..1cf242130b91 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1649,7 +1649,7 @@ 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, *sw;
 	int bus_type, mipi_lanes;
 
 	anx7625_get_swing_setting(dev, pdata);
@@ -1688,6 +1688,17 @@ 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.
+	 */
+	for_each_endpoint_of_node(np, sw) {
+		if (of_property_read_bool(sw, "mode-switch")) {
+			of_node_put(sw);
+			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.39.0.314.g84b9a713c41-goog

[PATCH v10 6/9] drm/bridge: anx7625: Register Type C mode switches

[Pin-yen Lin]

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>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>

---

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/Kconfig   |  1 +
 drivers/gpu/drm/bridge/analogix/anx7625.c | 92 +++++++++++++++++++++++
 drivers/gpu/drm/bridge/analogix/anx7625.h | 13 ++++
 3 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
index 173dada218ec..992b43ed1dd7 100644
--- a/drivers/gpu/drm/bridge/analogix/Kconfig
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -34,6 +34,7 @@ config DRM_ANALOGIX_ANX7625
 	tristate "Analogix Anx7625 MIPI to DP interface support"
 	depends on DRM
 	depends on OF
+	depends on TYPEC || TYPEC=n
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_DISPLAY_HELPER
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 1cf242130b91..808dbf79d209 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>
@@ -2572,6 +2574,90 @@ 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)
+{
+	struct drm_dp_typec_switch_desc switch_desc = ctx->switch_desc;
+	/* Check if both ports available and do nothing to retain the current one */
+	if (switch_desc.typec_ports[0].dp_connected && switch_desc.typec_ports[1].dp_connected)
+		return;
+
+	if (switch_desc.typec_ports[0].dp_connected)
+		anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL);
+	else if (switch_desc.typec_ports[1].dp_connected)
+		anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE);
+}
+
+static int anx7625_typec_mux_set(struct typec_mux_dev *mux,
+				 struct typec_mux_state *state)
+{
+	struct drm_dp_typec_port_data *port_data = typec_mux_get_drvdata(mux);
+	struct anx7625_data *ctx = (struct anx7625_data *) port_data->data;
+	struct device *dev = &ctx->client->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;
+
+	old_dp_connected = switch_desc.typec_ports[0].dp_connected ||
+			   switch_desc.typec_ports[1].dp_connected;
+
+	port_data->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",
+		switch_desc.typec_ports[0].dp_connected, switch_desc.typec_ports[1].dp_connected);
+
+	new_dp_connected = switch_desc.typec_ports[0].dp_connected ||
+			   switch_desc.typec_ports[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);
+}
+
+static int anx7625_register_typec_switches(struct device *dev, struct anx7625_data *ctx)
+{
+	struct device_node *port = of_graph_get_port_by_id(dev->of_node, 1);
+
+	/*
+	 * Currently, only mode switch is implemented.
+	 * TODO: Implement Type-C orientation switch for anx7625.
+	 */
+	return drm_dp_register_typec_switches(dev, port, &ctx->switch_desc,
+					      ctx, anx7625_typec_mux_set);
+}
+
 static int anx7625_i2c_probe(struct i2c_client *client)
 {
 	struct anx7625_data *platform;
@@ -2679,6 +2765,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))
@@ -2730,6 +2820,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 14f33d6be289..38abbd3d6b36 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 ********/
 
 /***************************************************************/
@@ -479,6 +491,7 @@ struct anx7625_data {
 	struct drm_connector *connector;
 	struct mipi_dsi_device *dsi;
 	struct drm_dp_aux aux;
+	struct drm_dp_typec_switch_desc switch_desc;
 };
 
 #endif  /* __ANX7625_H__ */
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH v10 7/9] dt-bindings: display: bridge: it6505: Add mode-switch support

[Pin-yen Lin]

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 v9)

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   | 93 ++++++++++++++++---
 1 file changed, 82 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
index b16a9d9127dd..94182f131f86 100644
--- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
@@ -77,20 +77,43 @@ properties:
         unevaluatedProperties: false
         description: Video port for DP output
 
-        properties:
-          endpoint:
+        patternProperties:
+          "^endpoint@[01]$":
             $ref: /schemas/graph.yaml#/$defs/endpoint-base
             unevaluatedProperties: false
 
             properties:
+              reg:
+                maxItems: 1
+
+              remote-endpoint: true
+
               data-lanes:
-                minItems: 1
-                uniqueItems: true
-                items:
-                  - enum: [ 0, 1 ]
-                  - const: 1
-                  - const: 2
-                  - const: 3
+                oneOf:
+                  - items:
+                      - enum: [0, 1, 2, 3]
+
+                  - items:
+                      - const: 0
+                      - const: 1
+
+                  - items:
+                      - const: 2
+                      - const: 3
+
+                  - items:
+                      - const: 0
+                      - const: 1
+                      - const: 2
+                      - const: 3
+
+              mode-switch:
+                type: boolean
+                description: Register this node as a Type-C mode switch or not.
+
+            required:
+              - reg
+              - remote-endpoint
 
     required:
       - port@0
@@ -102,7 +125,6 @@ required:
   - pwr18-supply
   - interrupts
   - reset-gpios
-  - extcon
   - ports
 
 additionalProperties: false
@@ -139,8 +161,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 +173,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 = <2 3>;
+                        remote-endpoint = <&typec_port1>;
+                    };
+                };
+            };
+        };
+    };
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH v10 8/9] drm/bridge: it6505: Fix Kconfig indentation

[Pin-yen Lin]

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>

---

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 57946d80b02d..737578dd57ed 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -85,19 +85,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.39.0.314.g84b9a713c41-goog

[PATCH v10 9/9] drm/bridge: it6505: Register Type C mode switches

[Pin-yen Lin]

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>

---

(no changes since v7)

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/Kconfig      |   1 +
 drivers/gpu/drm/bridge/ite-it6505.c | 119 +++++++++++++++++++++++++++-
 2 files changed, 116 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 737578dd57ed..33803f581562 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -87,6 +87,7 @@ config DRM_FSL_LDB
 config DRM_ITE_IT6505
 	tristate "ITE IT6505 DisplayPort bridge"
 	depends on OF
+	depends on TYPEC || TYPEC=n
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDCP_HELPER
 	select DRM_DISPLAY_HELPER
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index 9cda2df21b88..d9be09e889e2 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>
@@ -28,6 +30,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_of.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 
@@ -455,6 +458,7 @@ struct it6505 {
 	struct delayed_work delayed_audio;
 	struct it6505_audio_data audio;
 	struct dentry *debugfs;
+	struct drm_dp_typec_switch_desc switch_desc;
 
 	/* it6505 driver hold option */
 	bool enable_drv_hold;
@@ -3346,12 +3350,105 @@ static void it6505_shutdown(struct i2c_client *client)
 		it6505_lane_off(it6505);
 }
 
+static void it6505_typec_ports_update(struct it6505 *it6505)
+{
+	struct drm_dp_typec_switch_desc switch_desc = it6505->switch_desc;
+
+	/* Check if both ports available and do nothing to retain the current one */
+	if (switch_desc.typec_ports[0].dp_connected && switch_desc.typec_ports[1].dp_connected)
+		return;
+
+	if (switch_desc.typec_ports[0].dp_connected)
+		it6505->lane_swap = false;
+	else if (switch_desc.typec_ports[1].dp_connected)
+		it6505->lane_swap = true;
+}
+
+static int it6505_typec_mux_set(struct typec_mux_dev *mux,
+				struct typec_mux_state *state)
+{
+	struct drm_dp_typec_port_data *port_data = typec_mux_get_drvdata(mux);
+	struct it6505 *it6505 = (struct it6505 *) port_data->data;
+	struct device *dev = &it6505->client->dev;
+	struct drm_dp_typec_switch_desc switch_desc = it6505->switch_desc;
+	bool old_dp_connected, new_dp_connected;
+
+	if (switch_desc.num_typec_switches == 1)
+		return 0;
+
+	mutex_lock(&it6505->extcon_lock);
+
+	old_dp_connected = switch_desc.typec_ports[0].dp_connected ||
+			   switch_desc.typec_ports[1].dp_connected;
+
+	port_data->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",
+		switch_desc.typec_ports[0].dp_connected, switch_desc.typec_ports[1].dp_connected);
+
+	new_dp_connected = switch_desc.typec_ports[0].dp_connected ||
+			   switch_desc.typec_ports[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) {
+		int ret = pm_runtime_get_sync(dev);
+
+		/*
+		 * 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 (ret < 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 0;
+}
+
+static void it6505_unregister_typec_switches(struct it6505 *it6505)
+{
+	drm_dp_unregister_typec_switches(&it6505->switch_desc);
+}
+
+static int it6505_register_typec_switches(struct device *dev, struct it6505 *it6505)
+{
+	struct device_node *port = of_graph_get_port_by_id(dev->of_node, 1);
+
+	return drm_dp_register_typec_switches(dev, port, &it6505->switch_desc,
+					      it6505, it6505_typec_mux_set);
+}
+
 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)
@@ -3371,11 +3468,24 @@ static int it6505_i2c_probe(struct i2c_client *client)
 	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);
+		if (PTR_ERR(extcon) != -ENODEV)
+			dev_warn(dev, "Cannot get extcon device: %ld\n",
+				 PTR_ERR(extcon));
+		it6505->extcon = NULL;
+	} else {
+		it6505->extcon = extcon;
 	}
 
-	it6505->extcon = extcon;
+	ret = it6505_register_typec_switches(dev, it6505);
+	if (ret) {
+		if (ret != -ENODEV)
+			dev_warn(dev, "Didn't register Type-C switches, err: %d\n",
+				 ret);
+		if (!it6505->extcon) {
+			dev_err(dev, "Both extcon and typec-switch are not registered.\n");
+			return -EINVAL;
+		}
+	}
 
 	it6505->regmap = devm_regmap_init_i2c(client, &it6505_regmap_config);
 	if (IS_ERR(it6505->regmap)) {
@@ -3447,6 +3557,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.39.0.314.g84b9a713c41-goog

Re: [PATCH v10 9/9] drm/bridge: it6505: Register Type C mode switches

[Chen-Yu Tsai]

On Thu, Jan 12, 2023 at 12:22 PM Pin-yen Lin <treapking@chromium.org> wrote:
>
> 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>

Tested-by: Chen-Yu Tsai <wenst@chromium.org>

Re: [PATCH v10 0/9] Register Type-C mode-switch in DP bridge endpoints

[Dmitry Baryshkov]

On 12/01/2023 06:20, Pin-yen Lin wrote:
> 
> 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.

Please cc everybody on all patches. Having received just a single patch 
made me spend time on having to look them up on lore.

> 
> 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.
> 
> 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 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 (7):
>    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: 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      |  99 ++++++++++++-
>   .../bindings/display/bridge/ite,it6505.yaml   |  93 ++++++++++--
>   drivers/base/property.c                       |  15 ++
>   drivers/gpu/drm/bridge/Kconfig                |  21 +--
>   drivers/gpu/drm/bridge/analogix/Kconfig       |   1 +
>   drivers/gpu/drm/bridge/analogix/anx7625.c     | 105 +++++++++++++-
>   drivers/gpu/drm/bridge/analogix/anx7625.h     |  13 ++
>   drivers/gpu/drm/bridge/ite-it6505.c           | 119 +++++++++++++++-
>   drivers/gpu/drm/display/drm_dp_helper.c       | 134 ++++++++++++++++++
>   drivers/platform/chrome/cros_ec_typec.c       |  10 ++
>   include/drm/display/drm_dp_helper.h           |  17 +++
>   11 files changed, 598 insertions(+), 29 deletions(-)
> 

-- 
With best wishes
Dmitry

Re: [PATCH v10 0/9] Register Type-C mode-switch in DP bridge endpoints

[Pin-yen Lin]

Hi Dmitry,

On Thu, Jan 12, 2023 at 12:34 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 12/01/2023 06:20, Pin-yen Lin wrote:
> >
> > 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.
>
> Please cc everybody on all patches. Having received just a single patch
> made me spend time on having to look them up on lore.

Sorry about that. I'll prevent that in the future series.

>
> >
> > 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.
> >
> > 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 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 (7):
> >    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: 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      |  99 ++++++++++++-
> >   .../bindings/display/bridge/ite,it6505.yaml   |  93 ++++++++++--
> >   drivers/base/property.c                       |  15 ++
> >   drivers/gpu/drm/bridge/Kconfig                |  21 +--
> >   drivers/gpu/drm/bridge/analogix/Kconfig       |   1 +
> >   drivers/gpu/drm/bridge/analogix/anx7625.c     | 105 +++++++++++++-
> >   drivers/gpu/drm/bridge/analogix/anx7625.h     |  13 ++
> >   drivers/gpu/drm/bridge/ite-it6505.c           | 119 +++++++++++++++-
> >   drivers/gpu/drm/display/drm_dp_helper.c       | 134 ++++++++++++++++++
> >   drivers/platform/chrome/cros_ec_typec.c       |  10 ++
> >   include/drm/display/drm_dp_helper.h           |  17 +++
> >   11 files changed, 598 insertions(+), 29 deletions(-)
> >
>
> --
> With best wishes
> Dmitry
>

Thanks and regards,
Pin-yen

Re: [PATCH v10 0/9] Register Type-C mode-switch in DP bridge endpoints

[Andy Shevchenko]

On Thu, Jan 12, 2023 at 06:34:52AM +0200, Dmitry Baryshkov wrote:
> On 12/01/2023 06:20, Pin-yen Lin wrote:
> > 
> > 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.
> 
> Please cc everybody on all patches. Having received just a single patch made
> me spend time on having to look them up on lore.

Sorry but this seems a bit outdated requirement nowadays.
Why? Because we have `b4` in each famous Linux distro and
nice lore.kernel.org. It's really easy for the experienced
maintainer to catch up the whole thread.

Putting _all_ people on all patches may be an overkill.
What people should really get is the cover letter (assuming
that it explains well the structure of the series). The rest
depends.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v10 0/9] Register Type-C mode-switch in DP bridge endpoints

[Rob Herring]

On Wed, Jan 11, 2023 at 10:21 PM Pin-yen Lin <treapking@chromium.org> wrote:
>
>
> 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.
>
> 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.
>
> 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 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

4 versions in a week! Please slow down your pace. When you send a new
version, you move to the end of my review queue.

IIRC, these 2 chips are a bit different in what the mode switch or
muxing looks like. One had a built-in mux and the other doesn't? Do I
have to go research this again? No, you need to explain all this in
this series.

Rob

Re: [PATCH v10 0/9] Register Type-C mode-switch in DP bridge endpoints

[Pin-yen Lin]

Hi Rob,

On Fri, Jan 13, 2023 at 6:44 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Wed, Jan 11, 2023 at 10:21 PM Pin-yen Lin <treapking@chromium.org> wrote:
> >
> >
> > 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.
> >
> > 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.
> >
> > 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 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
>
> 4 versions in a week! Please slow down your pace. When you send a new
> version, you move to the end of my review queue.

I see. I'll keep this in mind in the future series.
>
> IIRC, these 2 chips are a bit different in what the mode switch or
> muxing looks like. One had a built-in mux and the other doesn't? Do I
> have to go research this again? No, you need to explain all this in
> this series.

Yes, anx7625 has a built-in mux while it6505 doesn't, but it's for
another use case.

IIUC the built-in mux in anx7625 is designed for automatically
switching between two orientations of a single Type-C connector, and
in that case we might need to register an orientation switch. But we
don't have hardware for this use case.

The use case this series aimed is having two downstreams for the
bridges, and registering two mode switches to switch between them. In
this use case, the built-in mux of anx7625 is not used and the
behavior of the switches is the same as it6505.

Explanations and TODOs have been added in the anx7625 driver change. I
can also mention this in the cover letter in the future series and
please let me know if anything is not clear for you.

>
> Rob

Thanks and regards,
Pin-yen