On Sat, 10 Feb 2024 at 09:14, Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
>
> We can imagine that logically the EC is a device that has some number of
> DisplayPort (DP) connector inputs, some number of USB3 connector inputs,
> and some number of USB type-c connector outputs. If you squint enough it
> looks like a USB type-c dock. Logically there's a crossbar pin
> assignment capability within the EC that can assign USB and DP lanes to
> USB type-c lanes in the connector (i.e. USB type-c pin configurations).
> In reality, the EC is a microcontroller that has some TCPCs and
> redrivers connected to it over something like i2c and DP/USB from the AP
> is wired directly to those ICs, not the EC.
>
> This design allows the EC to abstract many possible USB and DP hardware
> configurations away from the AP (kernel) so that the AP can largely deal
> with USB and DP without thinking about USB Type-C much at all. The DP
> and USB data originate in the AP, not the EC, so it helps to think that
> the EC takes the DP and USB data as input to mux onto USB type-c ports
> even if it really doesn't do that. With this split design, the EC
> forwards the DP HPD state to the AP via a GPIO that's connected to the
> DP phy.
>
> Having that HPD state signaled directly to the DP phy uses precious
> hardware resources, a GPIO or two and a wire, and it also forces the
> TCPM to live on the EC. If we want to save costs and move more control
> of USB type-c to the kernel it's in our interest to get rid of the HPD
> GPIO entirely and signal HPD to the DP phy some other way. Luckily, the
> EC already exposes information about the USB Type-C stack to the kernel
> via the host command interface in the "google,cros-ec-typec" compatible
> driver, which parses EC messages related to USB type-c and effectively
> "replays" those messages to the kernel's USB typec subsystem. This
> includes the state of HPD, which can be interrogated and acted upon by
> registering a 'struct typec_mux_dev' with the typec subsystem.
>
> On DT based systems, the DP display pipeline is abstracted via a 'struct
> drm_bridge'. If we want to signal HPD state within the kernel we need to
> hook into the drm_bridge framework somehow to call
> drm_bridge_hpd_notify() when HPD state changes in the typec framework.
> Make a drm_bridge in the EC that attaches onto the end of the DP bridge
> chain and logically moves the display data onto a usb-c-connector.
> Signal HPD when the typec HPD state changes, as long as this new
> drm_bridge is the one that's supposed to signal HPD. Do that by
> registering a 'struct typec_mux_dev' with the typec framework and
> associating that struct with a usb-c-connector node and a drm_bridge.
>
> To keep this patch minimal, just signal HPD state to the drm_bridge
> chain. Later patches will add more features. Eventually we'll be able to
> inform userspace about which usb-c-connector node is displaying DP and
> what USB devices are connected to a connector. Note that this code is
> placed in the cros_typec_switch driver because that's where mode-switch
> devices on the EC are controlled by the AP. Logically this drm_bridge
> sits in front of the mode-switch on the EC, and if there is anything to
> control on the EC the 'EC_FEATURE_TYPEC_AP_MUX_SET' feature will be set.
>
> Cc: Prashant Malani <pmalani@xxxxxxxxxxxx>
> Cc: Benson Leung <bleung@xxxxxxxxxxxx>
> Cc: Tzung-Bi Shih <tzungbi@xxxxxxxxxx>
> Cc: <chrome-platform@xxxxxxxxxxxxxxx>
> Cc: Pin-yen Lin <treapking@xxxxxxxxxxxx>
> Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> ---
> drivers/platform/chrome/Kconfig | 3 +-
> drivers/platform/chrome/cros_typec_switch.c | 218 ++++++++++++++++++--
> 2 files changed, 204 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 7a83346bfa53..910aa8be9c84 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -287,7 +287,8 @@ config CHROMEOS_PRIVACY_SCREEN
>
> config CROS_TYPEC_SWITCH
> tristate "ChromeOS EC Type-C Switch Control"
> - depends on MFD_CROS_EC_DEV && TYPEC && ACPI
> + depends on MFD_CROS_EC_DEV
> + depends on TYPEC
> default MFD_CROS_EC_DEV
> help
> If you say Y here, you get support for configuring the ChromeOS EC Type-C
> diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
> index 769de2889f2f..d8fb6662cf8d 100644
> --- a/drivers/platform/chrome/cros_typec_switch.c
> +++ b/drivers/platform/chrome/cros_typec_switch.c
> @@ -10,6 +10,7 @@
> #include <linux/delay.h>
> #include <linux/iopoll.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/platform_data/cros_ec_commands.h>
> #include <linux/platform_data/cros_ec_proto.h>
> #include <linux/platform_device.h>
> @@ -18,6 +19,15 @@
> #include <linux/usb/typec_mux.h>
> #include <linux/usb/typec_retimer.h>
>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_print.h>
> +
> +struct cros_typec_dp_bridge {
> + struct cros_typec_switch_data *sdata;
> + bool hpd_enabled;
> + struct drm_bridge bridge;
> +};
Is there any chance that you can use drm_dp_hpd_bridge_register() /
drm_aux_hpd_bridge_notify() instead of implementing another
drm_bridge?
If something is missing from the existing implementation we can
probably work that out.
> +
> /* Handles and other relevant data required for each port's switches. */
> struct cros_typec_port {
> int port_num;
> @@ -30,7 +40,9 @@ struct cros_typec_port {
> struct cros_typec_switch_data {
> struct device *dev;
> struct cros_ec_device *ec;
> + bool typec_cmd_supported;
> struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
> + struct cros_typec_dp_bridge *typec_dp_bridge;
> };
>
> static int cros_typec_cmd_mux_set(struct cros_typec_switch_data *sdata, int port_num, u8 index,
> @@ -143,13 +155,60 @@ static int cros_typec_configure_mux(struct cros_typec_switch_data *sdata, int po
> return 0;
> }
>
> +static int cros_typec_dp_port_switch_set(struct typec_mux_dev *mode_switch,
> + struct typec_mux_state *state)
> +{
> + struct cros_typec_port *port;
> + const struct typec_displayport_data *dp_data;
> + struct cros_typec_dp_bridge *typec_dp_bridge;
> + struct drm_bridge *bridge;
> + bool hpd_asserted;
> +
> + port = typec_mux_get_drvdata(mode_switch);
> + typec_dp_bridge = port->sdata->typec_dp_bridge;
> + if (!typec_dp_bridge)
> + return 0;
> +
> + bridge = &typec_dp_bridge->bridge;
> +
> + if (state->mode == TYPEC_STATE_SAFE || state->mode == TYPEC_STATE_USB) {
> + if (typec_dp_bridge->hpd_enabled)
> + drm_bridge_hpd_notify(bridge, connector_status_disconnected);
> +
> + return 0;
> + }
> +
> + if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
> + if (typec_dp_bridge->hpd_enabled) {
> + dp_data = state->data;
> + hpd_asserted = dp_data->status & DP_STATUS_HPD_STATE;
> +
> + if (hpd_asserted)
> + drm_bridge_hpd_notify(bridge, connector_status_connected);
> + else
> + drm_bridge_hpd_notify(bridge, connector_status_disconnected);
> + }
> + }
> +
> + return 0;
> +}
> +
> static int cros_typec_mode_switch_set(struct typec_mux_dev *mode_switch,
> struct typec_mux_state *state)
> {
> struct cros_typec_port *port = typec_mux_get_drvdata(mode_switch);
> + struct cros_typec_switch_data *sdata = port->sdata;
> + int ret;
> +
> + ret = cros_typec_dp_port_switch_set(mode_switch, state);
> + if (ret)
> + return ret;
>
> /* Mode switches have index 0. */
> - return cros_typec_configure_mux(port->sdata, port->port_num, 0, state->mode, state->alt);
> + if (sdata->typec_cmd_supported)
> + return cros_typec_configure_mux(port->sdata, port->port_num, 0, state->mode, state->alt);
> +
> + return 0;
> }
>
> static int cros_typec_retimer_set(struct typec_retimer *retimer, struct typec_retimer_state *state)
> @@ -201,12 +260,77 @@ static int cros_typec_register_retimer(struct cros_typec_port *port, struct fwno
> return PTR_ERR_OR_ZERO(port->retimer);
> }
>
> +static int
> +cros_typec_dp_bridge_attach(struct drm_bridge *bridge,
> + enum drm_bridge_attach_flags flags)
> +{
> + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> + DRM_ERROR("Fix bridge driver to make connector optional!\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static struct cros_typec_dp_bridge *
> +bridge_to_cros_typec_dp_bridge(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct cros_typec_dp_bridge, bridge);
> +}
> +
> +static void cros_typec_dp_bridge_hpd_enable(struct drm_bridge *bridge)
> +{
> + struct cros_typec_dp_bridge *typec_dp_bridge;
> +
> + typec_dp_bridge = bridge_to_cros_typec_dp_bridge(bridge);
> + typec_dp_bridge->hpd_enabled = true;
> +}
> +
> +static void cros_typec_dp_bridge_hpd_disable(struct drm_bridge *bridge)
> +{
> + struct cros_typec_dp_bridge *typec_dp_bridge;
> +
> + typec_dp_bridge = bridge_to_cros_typec_dp_bridge(bridge);
> + typec_dp_bridge->hpd_enabled = false;
> +}
> +
> +static const struct drm_bridge_funcs cros_typec_dp_bridge_funcs = {
> + .attach = cros_typec_dp_bridge_attach,
> + .hpd_enable = cros_typec_dp_bridge_hpd_enable,
> + .hpd_disable = cros_typec_dp_bridge_hpd_disable,
> +};
> +
> +static int cros_typec_register_dp_bridge(struct cros_typec_switch_data *sdata,
> + struct fwnode_handle *fwnode)
> +{
> + struct cros_typec_dp_bridge *typec_dp_bridge;
> + struct drm_bridge *bridge;
> + struct device *dev = sdata->dev;
> +
> + typec_dp_bridge = devm_kzalloc(dev, sizeof(*typec_dp_bridge), GFP_KERNEL);
> + if (!typec_dp_bridge)
> + return -ENOMEM;
> +
> + typec_dp_bridge->sdata = sdata;
> + sdata->typec_dp_bridge = typec_dp_bridge;
> + bridge = &typec_dp_bridge->bridge;
> +
> + bridge->funcs = &cros_typec_dp_bridge_funcs;
> + bridge->of_node = dev->of_node;
> + bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
> + bridge->ops |= DRM_BRIDGE_OP_HPD;
> +
> + return devm_drm_bridge_add(dev, bridge);
> +}
> +
> static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
> struct fwnode_handle *fwnode)
> {
> struct cros_typec_port *port;
> struct device *dev = sdata->dev;
> struct acpi_device *adev;
> + struct device_node *np;
> + struct fwnode_handle *port_node;
> u32 index;
> int ret;
> const char *prop_name;
> @@ -218,9 +342,12 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
> adev = to_acpi_device_node(fwnode);
> if (adev)
> prop_name = "_ADR";
> + np = to_of_node(fwnode);
> + if (np)
> + prop_name = "reg";
>
> - if (!adev)
> - return dev_err_probe(fwnode->dev, -ENODEV, "Couldn't get ACPI handle\n");
> + if (!adev && !np)
> + return dev_err_probe(fwnode->dev, -ENODEV, "Couldn't get ACPI/OF device handle\n");
>
> ret = fwnode_property_read_u32(fwnode, prop_name, &index);
> if (ret)
> @@ -232,41 +359,84 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
> port->port_num = index;
> sdata->ports[index] = port;
>
> + port_node = fwnode;
> + if (np)
> + fwnode = fwnode_graph_get_port_parent(fwnode);
> +
> if (fwnode_property_present(fwnode, "retimer-switch")) {
> - ret = cros_typec_register_retimer(port, fwnode);
> - if (ret)
> - return dev_err_probe(dev, ret, "Retimer switch register failed\n");
> + ret = cros_typec_register_retimer(port, port_node);
> + if (ret) {
> + dev_err_probe(dev, ret, "Retimer switch register failed\n");
> + goto out;
> + }
>
> dev_dbg(dev, "Retimer switch registered for index %u\n", index);
> }
>
> - if (!fwnode_property_present(fwnode, "mode-switch"))
> - return 0;
> + if (fwnode_property_present(fwnode, "mode-switch")) {
> + ret = cros_typec_register_mode_switch(port, port_node);
> + if (ret) {
> + dev_err_probe(dev, ret, "Mode switch register failed\n");
> + goto out;
> + }
>
> - ret = cros_typec_register_mode_switch(port, fwnode);
> - if (ret)
> - return dev_err_probe(dev, ret, "Mode switch register failed\n");
> + dev_dbg(dev, "Mode switch registered for index %u\n", index);
> + }
>
> - dev_dbg(dev, "Mode switch registered for index %u\n", index);
>
> +out:
> + if (np)
> + fwnode_handle_put(fwnode);
> return ret;
> }
>
> static int cros_typec_register_switches(struct cros_typec_switch_data *sdata)
> {
> struct device *dev = sdata->dev;
> + struct fwnode_handle *devnode;
> struct fwnode_handle *fwnode;
> + struct fwnode_endpoint endpoint;
> int nports, ret;
>
> nports = device_get_child_node_count(dev);
> if (nports == 0)
> return dev_err_probe(dev, -ENODEV, "No switch devices found\n");
>
> - device_for_each_child_node(dev, fwnode) {
> - ret = cros_typec_register_port(sdata, fwnode);
> - if (ret) {
> + devnode = dev_fwnode(dev);
> + if (fwnode_graph_get_endpoint_count(devnode, 0)) {
> + fwnode_graph_for_each_endpoint(devnode, fwnode) {
> + ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
> + if (ret) {
> + fwnode_handle_put(fwnode);
> + goto err;
> + }
> + /* Skip if not a type-c output port */
> + if (endpoint.port != 2)
> + continue;
> +
> + ret = cros_typec_register_port(sdata, fwnode);
> + if (ret) {
> + fwnode_handle_put(fwnode);
> + goto err;
> + }
> + }
> + } else {
> + device_for_each_child_node(dev, fwnode) {
> + ret = cros_typec_register_port(sdata, fwnode);
> + if (ret) {
> + fwnode_handle_put(fwnode);
> + goto err;
> + }
> + }
> + }
> +
> + if (fwnode_property_present(devnode, "mode-switch")) {
> + fwnode = fwnode_graph_get_endpoint_by_id(devnode, 0, 0, 0);
> + if (fwnode) {
> + ret = cros_typec_register_dp_bridge(sdata, fwnode);
> fwnode_handle_put(fwnode);
> - goto err;
> + if (ret)
> + goto err;
> }
> }
>
> @@ -280,6 +450,7 @@ static int cros_typec_switch_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct cros_typec_switch_data *sdata;
> + struct cros_ec_dev *ec_dev;
>
> sdata = devm_kzalloc(dev, sizeof(*sdata), GFP_KERNEL);
> if (!sdata)
> @@ -288,6 +459,12 @@ static int cros_typec_switch_probe(struct platform_device *pdev)
> sdata->dev = dev;
> sdata->ec = dev_get_drvdata(pdev->dev.parent);
>
> + ec_dev = dev_get_drvdata(&sdata->ec->ec->dev);
> + if (!ec_dev)
> + return -EPROBE_DEFER;
> +
> + sdata->typec_cmd_supported = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_AP_MUX_SET);
> +
> platform_set_drvdata(pdev, sdata);
>
> return cros_typec_register_switches(sdata);
> @@ -308,10 +485,19 @@ static const struct acpi_device_id cros_typec_switch_acpi_id[] = {
> MODULE_DEVICE_TABLE(acpi, cros_typec_switch_acpi_id);
> #endif
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id cros_typec_switch_of_match_table[] = {
> + { .compatible = "google,cros-ec-typec-switch" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, cros_typec_switch_of_match_table);
> +#endif
> +
> static struct platform_driver cros_typec_switch_driver = {
> .driver = {
> .name = "cros-typec-switch",
> .acpi_match_table = ACPI_PTR(cros_typec_switch_acpi_id),
> + .of_match_table = of_match_ptr(cros_typec_switch_of_match_table),
> },
> .probe = cros_typec_switch_probe,
> .remove_new = cros_typec_switch_remove,
> --
> https://chromeos.dev
>
>
--
With best wishes
Dmitry
