| Message ID | 20230112042104.4107253-4-treapking@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Register Type-C mode-switch in DP bridge endpoints | expand |
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_ */
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
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
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
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
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
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_ */
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
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,
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.
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_ */