Greg, could you please ack the last patch to be merged through the
drm-misc tree? You have acked patch 3, but since that time I've added
patches 4-6.
Supporting DP/USB-C can result in a chain of several transparent
bridges (PHY, redrivers, mux, etc). All attempts to implement DP support
in a different way resulted either in series of hacks or in device tree
not reflecting the actual hardware design. This results in drivers
having similar boilerplate code for such bridges.
Next, these drivers are susceptible to -EPROBE_DEFER loops: the next
bridge can either be probed from the bridge->attach callback, when it is
too late to return -EPROBE_DEFER, or from the probe() callback, when the
next bridge might not yet be available, because it depends on the
resources provided by the probing device. Device links can not fully
solve this problem since there are mutual dependencies between adjancent
devices.
Last, but not least, this results in the the internal knowledge of DRM
subsystem slowly diffusing into other subsystems, like PHY or USB/TYPEC.
To solve all these issues, define a separate DRM helper, which creates
separate aux device just for the bridge. During probe such aux device
doesn't result in the EPROBE_DEFER loops. Instead it allows the device
drivers to probe properly, according to the actual resource
dependencies. The bridge auxdevs are then probed when the next bridge
becomes available, sparing drivers from drm_bridge_attach() returning
-EPROBE_DEFER.
Changes since v5:
- Removed extra semicolon in !DRM_AUX_HPD_BRIDGE stubs definition.
Changes since v4:
- Added documentation for new API (Sima)
- Added generic code to handle "last mile" DP bridges implementing just
the HPD functionality.
- Rebased on top of linux-next to be able to drop #ifdef's around
drm_bridge->of_node
Changes since v3:
- Moved bridge driver to gpu/drm/bridge (Neil Armstrong)
- Renamed it to aux-bridge (since there is already a simple_bridge driver)
- Made CONFIG_OF mandatory for this driver (Neil Armstrong)
- Added missing kfree and ida_free (Dan Carpenter)
Changes since v2:
- ifdef'ed bridge->of_node access (LKP)
Changes since v1:
- Added EXPORT_SYMBOL_GPL / MODULE_LICENSE / etc. to drm_simple_bridge
Dmitry Baryshkov (6):
drm/bridge: add transparent bridge helper
phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE
usb: typec: nb7vpq904m: switch to DRM_AUX_BRIDGE
drm/bridge: implement generic DP HPD bridge
soc: qcom: pmic-glink: switch to DRM_AUX_HPD_BRIDGE
usb: typec: qcom-pmic-typec: switch to DRM_AUX_HPD_BRIDGE
drivers/gpu/drm/bridge/Kconfig | 17 ++
drivers/gpu/drm/bridge/Makefile | 2 +
drivers/gpu/drm/bridge/aux-bridge.c | 140 +++++++++++++++
drivers/gpu/drm/bridge/aux-hpd-bridge.c | 164 ++++++++++++++++++
drivers/phy/qualcomm/Kconfig | 2 +-
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 44 +----
drivers/soc/qcom/Kconfig | 1 +
drivers/soc/qcom/pmic_glink_altmode.c | 33 +---
drivers/usb/typec/mux/Kconfig | 2 +-
drivers/usb/typec/mux/nb7vpq904m.c | 44 +----
drivers/usb/typec/tcpm/Kconfig | 1 +
drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 41 +----
include/drm/bridge/aux-bridge.h | 37 ++++
13 files changed, 383 insertions(+), 145 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/aux-bridge.c
create mode 100644 drivers/gpu/drm/bridge/aux-hpd-bridge.c
create mode 100644 include/drm/bridge/aux-bridge.h
--
2.39.2
Switch to using the new DRM_AUX_BRIDGE helper to create the
transparent DRM bridge device instead of handcoding corresponding
functionality.
Reviewed-by: Heikki Krogerus <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/usb/typec/mux/Kconfig | 2 +-
drivers/usb/typec/mux/nb7vpq904m.c | 44 ++----------------------------
2 files changed, 3 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
index 816b9bd08355..5120942f309d 100644
--- a/drivers/usb/typec/mux/Kconfig
+++ b/drivers/usb/typec/mux/Kconfig
@@ -40,7 +40,7 @@ config TYPEC_MUX_NB7VPQ904M
tristate "On Semiconductor NB7VPQ904M Type-C redriver driver"
depends on I2C
depends on DRM || DRM=n
- select DRM_PANEL_BRIDGE if DRM
+ select DRM_AUX_BRIDGE if DRM_BRIDGE
select REGMAP_I2C
help
Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C
diff --git a/drivers/usb/typec/mux/nb7vpq904m.c b/drivers/usb/typec/mux/nb7vpq904m.c
index cda206cf0c38..b17826713753 100644
--- a/drivers/usb/typec/mux/nb7vpq904m.c
+++ b/drivers/usb/typec/mux/nb7vpq904m.c
@@ -11,7 +11,7 @@
#include <linux/regmap.h>
#include <linux/bitfield.h>
#include <linux/of_graph.h>
-#include <drm/drm_bridge.h>
+#include <drm/bridge/aux-bridge.h>
#include <linux/usb/typec_dp.h>
#include <linux/usb/typec_mux.h>
#include <linux/usb/typec_retimer.h>
@@ -70,8 +70,6 @@ struct nb7vpq904m {
bool swap_data_lanes;
struct typec_switch *typec_switch;
- struct drm_bridge bridge;
-
struct mutex lock; /* protect non-concurrent retimer & switch */
enum typec_orientation orientation;
@@ -297,44 +295,6 @@ static int nb7vpq904m_retimer_set(struct typec_retimer *retimer, struct typec_re
return ret;
}
-#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_PANEL_BRIDGE)
-static int nb7vpq904m_bridge_attach(struct drm_bridge *bridge,
- enum drm_bridge_attach_flags flags)
-{
- struct nb7vpq904m *nb7 = container_of(bridge, struct nb7vpq904m, bridge);
- struct drm_bridge *next_bridge;
-
- if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
- return -EINVAL;
-
- next_bridge = devm_drm_of_get_bridge(&nb7->client->dev, nb7->client->dev.of_node, 0, 0);
- if (IS_ERR(next_bridge)) {
- dev_err(&nb7->client->dev, "failed to acquire drm_bridge: %pe\n", next_bridge);
- return PTR_ERR(next_bridge);
- }
-
- return drm_bridge_attach(bridge->encoder, next_bridge, bridge,
- DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-}
-
-static const struct drm_bridge_funcs nb7vpq904m_bridge_funcs = {
- .attach = nb7vpq904m_bridge_attach,
-};
-
-static int nb7vpq904m_register_bridge(struct nb7vpq904m *nb7)
-{
- nb7->bridge.funcs = &nb7vpq904m_bridge_funcs;
- nb7->bridge.of_node = nb7->client->dev.of_node;
-
- return devm_drm_bridge_add(&nb7->client->dev, &nb7->bridge);
-}
-#else
-static int nb7vpq904m_register_bridge(struct nb7vpq904m *nb7)
-{
- return 0;
-}
-#endif
-
static const struct regmap_config nb7_regmap = {
.max_register = 0x1f,
.reg_bits = 8,
@@ -461,7 +421,7 @@ static int nb7vpq904m_probe(struct i2c_client *client)
gpiod_set_value(nb7->enable_gpio, 1);
- ret = nb7vpq904m_register_bridge(nb7);
+ ret = drm_aux_bridge_register(dev);
if (ret)
goto err_disable_gpio;
--
2.39.2
Switch to using the new DRM_AUX_BRIDGE helper to create the
transparent DRM bridge device instead of handcoding corresponding
functionality.
Acked-by: Vinod Koul <[email protected]>
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/phy/qualcomm/Kconfig | 2 +-
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 44 ++---------------------
2 files changed, 3 insertions(+), 43 deletions(-)
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index d891058b7c39..846f8c99547f 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -63,7 +63,7 @@ config PHY_QCOM_QMP_COMBO
depends on DRM || DRM=n
select GENERIC_PHY
select MFD_SYSCON
- select DRM_PANEL_BRIDGE if DRM
+ select DRM_AUX_BRIDGE if DRM_BRIDGE
help
Enable this to support the QMP Combo PHY transceiver that is used
with USB3 and DisplayPort controllers on Qualcomm chips.
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 0417856b8e7b..435cd849e82e 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -21,7 +21,7 @@
#include <linux/usb/typec.h>
#include <linux/usb/typec_mux.h>
-#include <drm/drm_bridge.h>
+#include <drm/bridge/aux-bridge.h>
#include <dt-bindings/phy/phy-qcom-qmp.h>
@@ -1419,8 +1419,6 @@ struct qmp_combo {
struct clk_hw dp_link_hw;
struct clk_hw dp_pixel_hw;
- struct drm_bridge bridge;
-
struct typec_switch_dev *sw;
enum typec_orientation orientation;
};
@@ -3191,44 +3189,6 @@ static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
}
#endif
-#if IS_ENABLED(CONFIG_DRM)
-static int qmp_combo_bridge_attach(struct drm_bridge *bridge,
- enum drm_bridge_attach_flags flags)
-{
- struct qmp_combo *qmp = container_of(bridge, struct qmp_combo, bridge);
- struct drm_bridge *next_bridge;
-
- if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
- return -EINVAL;
-
- next_bridge = devm_drm_of_get_bridge(qmp->dev, qmp->dev->of_node, 0, 0);
- if (IS_ERR(next_bridge)) {
- dev_err(qmp->dev, "failed to acquire drm_bridge: %pe\n", next_bridge);
- return PTR_ERR(next_bridge);
- }
-
- return drm_bridge_attach(bridge->encoder, next_bridge, bridge,
- DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-}
-
-static const struct drm_bridge_funcs qmp_combo_bridge_funcs = {
- .attach = qmp_combo_bridge_attach,
-};
-
-static int qmp_combo_dp_register_bridge(struct qmp_combo *qmp)
-{
- qmp->bridge.funcs = &qmp_combo_bridge_funcs;
- qmp->bridge.of_node = qmp->dev->of_node;
-
- return devm_drm_bridge_add(qmp->dev, &qmp->bridge);
-}
-#else
-static int qmp_combo_dp_register_bridge(struct qmp_combo *qmp)
-{
- return 0;
-}
-#endif
-
static int qmp_combo_parse_dt_lecacy_dp(struct qmp_combo *qmp, struct device_node *np)
{
struct device *dev = qmp->dev;
@@ -3440,7 +3400,7 @@ static int qmp_combo_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = qmp_combo_dp_register_bridge(qmp);
+ ret = drm_aux_bridge_register(dev);
if (ret)
return ret;
--
2.39.2
Define a helper for creating simple transparent bridges which serve the
only purpose of linking devices into the bridge chain up to the last
bridge representing the connector. This is especially useful for
DP/USB-C bridge chains, which can span across several devices, but do
not require any additional functionality from the intermediate bridges.
Reviewed-by: Neil Armstrong <[email protected]>
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/bridge/Kconfig | 9 ++
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/aux-bridge.c | 140 ++++++++++++++++++++++++++++
include/drm/bridge/aux-bridge.h | 19 ++++
4 files changed, 169 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/aux-bridge.c
create mode 100644 include/drm/bridge/aux-bridge.h
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index ba82a1142adf..f12eab62799f 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -12,6 +12,15 @@ config DRM_PANEL_BRIDGE
help
DRM bridge wrapper of DRM panels
+config DRM_AUX_BRIDGE
+ tristate
+ depends on DRM_BRIDGE && OF
+ select AUXILIARY_BUS
+ select DRM_PANEL_BRIDGE
+ help
+ Simple transparent bridge that is used by several non-DRM drivers to
+ build bridges chain.
+
menu "Display Interface Bridges"
depends on DRM && DRM_BRIDGE
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 2b892b7ed59e..918e3bfff079 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_DRM_AUX_BRIDGE) += aux-bridge.o
obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o
obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o
diff --git a/drivers/gpu/drm/bridge/aux-bridge.c b/drivers/gpu/drm/bridge/aux-bridge.c
new file mode 100644
index 000000000000..6245976b8fef
--- /dev/null
+++ b/drivers/gpu/drm/bridge/aux-bridge.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Author: Dmitry Baryshkov <[email protected]>
+ */
+#include <linux/auxiliary_bus.h>
+#include <linux/module.h>
+
+#include <drm/drm_bridge.h>
+#include <drm/bridge/aux-bridge.h>
+
+static DEFINE_IDA(drm_aux_bridge_ida);
+
+static void drm_aux_bridge_release(struct device *dev)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+ ida_free(&drm_aux_bridge_ida, adev->id);
+
+ kfree(adev);
+}
+
+static void drm_aux_bridge_unregister_adev(void *_adev)
+{
+ struct auxiliary_device *adev = _adev;
+
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+}
+
+/**
+ * drm_aux_bridge_register - Create a simple bridge device to link the chain
+ * @parent: device instance providing this bridge
+ *
+ * Creates a simple DRM bridge that doesn't implement any drm_bridge
+ * operations. Such bridges merely fill a place in the bridge chain linking
+ * surrounding DRM bridges.
+ *
+ * Return: zero on success, negative error code on failure
+ */
+int drm_aux_bridge_register(struct device *parent)
+{
+ struct auxiliary_device *adev;
+ int ret;
+
+ adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+ if (!adev)
+ return -ENOMEM;
+
+ ret = ida_alloc(&drm_aux_bridge_ida, GFP_KERNEL);
+ if (ret < 0) {
+ kfree(adev);
+ return ret;
+ }
+
+ adev->id = ret;
+ adev->name = "aux_bridge";
+ adev->dev.parent = parent;
+ adev->dev.of_node = parent->of_node;
+ adev->dev.release = drm_aux_bridge_release;
+
+ ret = auxiliary_device_init(adev);
+ if (ret) {
+ ida_free(&drm_aux_bridge_ida, adev->id);
+ kfree(adev);
+ return ret;
+ }
+
+ ret = auxiliary_device_add(adev);
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ return ret;
+ }
+
+ return devm_add_action_or_reset(parent, drm_aux_bridge_unregister_adev, adev);
+}
+EXPORT_SYMBOL_GPL(drm_aux_bridge_register);
+
+struct drm_aux_bridge_data {
+ struct drm_bridge bridge;
+ struct drm_bridge *next_bridge;
+ struct device *dev;
+};
+
+static int drm_aux_bridge_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct drm_aux_bridge_data *data;
+
+ if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
+ return -EINVAL;
+
+ data = container_of(bridge, struct drm_aux_bridge_data, bridge);
+
+ return drm_bridge_attach(bridge->encoder, data->next_bridge, bridge,
+ DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+}
+
+static const struct drm_bridge_funcs drm_aux_bridge_funcs = {
+ .attach = drm_aux_bridge_attach,
+};
+
+static int drm_aux_bridge_probe(struct auxiliary_device *auxdev,
+ const struct auxiliary_device_id *id)
+{
+ struct drm_aux_bridge_data *data;
+
+ data = devm_kzalloc(&auxdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->dev = &auxdev->dev;
+ data->next_bridge = devm_drm_of_get_bridge(&auxdev->dev, auxdev->dev.of_node, 0, 0);
+ if (IS_ERR(data->next_bridge))
+ return dev_err_probe(&auxdev->dev, PTR_ERR(data->next_bridge),
+ "failed to acquire drm_bridge\n");
+
+ data->bridge.funcs = &drm_aux_bridge_funcs;
+ data->bridge.of_node = data->dev->of_node;
+
+ return devm_drm_bridge_add(data->dev, &data->bridge);
+}
+
+static const struct auxiliary_device_id drm_aux_bridge_table[] = {
+ { .name = KBUILD_MODNAME ".aux_bridge" },
+ {},
+};
+MODULE_DEVICE_TABLE(auxiliary, drm_aux_bridge_table);
+
+static struct auxiliary_driver drm_aux_bridge_drv = {
+ .name = "aux_bridge",
+ .id_table = drm_aux_bridge_table,
+ .probe = drm_aux_bridge_probe,
+};
+module_auxiliary_driver(drm_aux_bridge_drv);
+
+MODULE_AUTHOR("Dmitry Baryshkov <[email protected]>");
+MODULE_DESCRIPTION("DRM transparent bridge");
+MODULE_LICENSE("GPL");
diff --git a/include/drm/bridge/aux-bridge.h b/include/drm/bridge/aux-bridge.h
new file mode 100644
index 000000000000..441ab3f0e920
--- /dev/null
+++ b/include/drm/bridge/aux-bridge.h
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Author: Dmitry Baryshkov <[email protected]>
+ */
+#ifndef DRM_AUX_BRIDGE_H
+#define DRM_AUX_BRIDGE_H
+
+#if IS_ENABLED(CONFIG_DRM_AUX_BRIDGE)
+int drm_aux_bridge_register(struct device *parent);
+#else
+static inline int drm_aux_bridge_register(struct device *parent)
+{
+ return 0;
+}
+#endif
+
+#endif
--
2.39.2
Several USB-C controllers implement a pretty simple DRM bridge which
implements just the HPD notification operations. Add special helper
for creating such simple bridges.
Acked-by: Neil Armstrong <[email protected]>
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/bridge/Kconfig | 8 ++
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/aux-hpd-bridge.c | 164 ++++++++++++++++++++++++
include/drm/bridge/aux-bridge.h | 18 +++
4 files changed, 191 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/aux-hpd-bridge.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index f12eab62799f..19d2dc05c397 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -21,6 +21,14 @@ config DRM_AUX_BRIDGE
Simple transparent bridge that is used by several non-DRM drivers to
build bridges chain.
+config DRM_AUX_HPD_BRIDGE
+ tristate
+ depends on DRM_BRIDGE && OF
+ select AUXILIARY_BUS
+ help
+ Simple bridge that terminates the bridge chain and provides HPD
+ support.
+
menu "Display Interface Bridges"
depends on DRM && DRM_BRIDGE
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 918e3bfff079..017b5832733b 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_DRM_AUX_BRIDGE) += aux-bridge.o
+obj-$(CONFIG_DRM_AUX_HPD_BRIDGE) += aux-hpd-bridge.o
obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o
obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o
diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
new file mode 100644
index 000000000000..4defac8ec63f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Author: Dmitry Baryshkov <[email protected]>
+ */
+#include <linux/auxiliary_bus.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+#include <drm/drm_bridge.h>
+#include <drm/bridge/aux-bridge.h>
+
+static DEFINE_IDA(drm_aux_hpd_bridge_ida);
+
+struct drm_aux_hpd_bridge_data {
+ struct drm_bridge bridge;
+ struct device *dev;
+};
+
+static void drm_aux_hpd_bridge_release(struct device *dev)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+ ida_free(&drm_aux_hpd_bridge_ida, adev->id);
+
+ of_node_put(adev->dev.platform_data);
+
+ kfree(adev);
+}
+
+static void drm_aux_hpd_bridge_unregister_adev(void *_adev)
+{
+ struct auxiliary_device *adev = _adev;
+
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+}
+
+/**
+ * drm_dp_hpd_bridge_register - Create a simple HPD DisplayPort bridge
+ * @parent: device instance providing this bridge
+ * @np: device node pointer corresponding to this bridge instance
+ *
+ * Creates a simple DRM bridge with the type set to
+ * DRM_MODE_CONNECTOR_DisplayPort, which terminates the bridge chain and is
+ * able to send the HPD events.
+ *
+ * Return: device instance that will handle created bridge or an error code
+ * encoded into the pointer.
+ */
+struct device *drm_dp_hpd_bridge_register(struct device *parent,
+ struct device_node *np)
+{
+ struct auxiliary_device *adev;
+ int ret;
+
+ adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+ if (!adev)
+ return ERR_PTR(-ENOMEM);
+
+ ret = ida_alloc(&drm_aux_hpd_bridge_ida, GFP_KERNEL);
+ if (ret < 0) {
+ kfree(adev);
+ return ERR_PTR(ret);
+ }
+
+ adev->id = ret;
+ adev->name = "dp_hpd_bridge";
+ adev->dev.parent = parent;
+ adev->dev.of_node = parent->of_node;
+ adev->dev.release = drm_aux_hpd_bridge_release;
+ adev->dev.platform_data = np;
+
+ ret = auxiliary_device_init(adev);
+ if (ret) {
+ ida_free(&drm_aux_hpd_bridge_ida, adev->id);
+ kfree(adev);
+ return ERR_PTR(ret);
+ }
+
+ ret = auxiliary_device_add(adev);
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ return ERR_PTR(ret);
+ }
+
+ ret = devm_add_action_or_reset(parent, drm_aux_hpd_bridge_unregister_adev, adev);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return &adev->dev;
+
+}
+EXPORT_SYMBOL_GPL(drm_dp_hpd_bridge_register);
+
+/**
+ * drm_aux_hpd_bridge_notify - notify hot plug detection events
+ * @dev: device created for the HPD bridge
+ * @status: output connection status
+ *
+ * A wrapper around drm_bridge_hpd_notify() that is used to report hot plug
+ * detection events for bridges created via drm_dp_hpd_bridge_register().
+ *
+ * This function shall be called in a context that can sleep.
+ */
+void drm_aux_hpd_bridge_notify(struct device *dev, enum drm_connector_status status)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+ struct drm_aux_hpd_bridge_data *data = auxiliary_get_drvdata(adev);
+
+ if (!data)
+ return;
+
+ drm_bridge_hpd_notify(&data->bridge, status);
+}
+EXPORT_SYMBOL_GPL(drm_aux_hpd_bridge_notify);
+
+static int drm_aux_hpd_bridge_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
+}
+
+static const struct drm_bridge_funcs drm_aux_hpd_bridge_funcs = {
+ .attach = drm_aux_hpd_bridge_attach,
+};
+
+static int drm_aux_hpd_bridge_probe(struct auxiliary_device *auxdev,
+ const struct auxiliary_device_id *id)
+{
+ struct drm_aux_hpd_bridge_data *data;
+
+ data = devm_kzalloc(&auxdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->dev = &auxdev->dev;
+ data->bridge.funcs = &drm_aux_hpd_bridge_funcs;
+ data->bridge.of_node = dev_get_platdata(data->dev);
+ data->bridge.ops = DRM_BRIDGE_OP_HPD;
+ data->bridge.type = id->driver_data;
+
+ auxiliary_set_drvdata(auxdev, data);
+
+ return devm_drm_bridge_add(data->dev, &data->bridge);
+}
+
+static const struct auxiliary_device_id drm_aux_hpd_bridge_table[] = {
+ { .name = KBUILD_MODNAME ".dp_hpd_bridge", .driver_data = DRM_MODE_CONNECTOR_DisplayPort, },
+ {},
+};
+MODULE_DEVICE_TABLE(auxiliary, drm_aux_hpd_bridge_table);
+
+static struct auxiliary_driver drm_aux_hpd_bridge_drv = {
+ .name = "aux_hpd_bridge",
+ .id_table = drm_aux_hpd_bridge_table,
+ .probe = drm_aux_hpd_bridge_probe,
+};
+module_auxiliary_driver(drm_aux_hpd_bridge_drv);
+
+MODULE_AUTHOR("Dmitry Baryshkov <[email protected]>");
+MODULE_DESCRIPTION("DRM HPD bridge");
+MODULE_LICENSE("GPL");
diff --git a/include/drm/bridge/aux-bridge.h b/include/drm/bridge/aux-bridge.h
index 441ab3f0e920..33adaf4e4daa 100644
--- a/include/drm/bridge/aux-bridge.h
+++ b/include/drm/bridge/aux-bridge.h
@@ -7,6 +7,8 @@
#ifndef DRM_AUX_BRIDGE_H
#define DRM_AUX_BRIDGE_H
+#include <drm/drm_connector.h>
+
#if IS_ENABLED(CONFIG_DRM_AUX_BRIDGE)
int drm_aux_bridge_register(struct device *parent);
#else
@@ -16,4 +18,20 @@ static inline int drm_aux_bridge_register(struct device *parent)
}
#endif
+#if IS_ENABLED(CONFIG_DRM_AUX_HPD_BRIDGE)
+struct device *drm_dp_hpd_bridge_register(struct device *parent,
+ struct device_node *np);
+void drm_aux_hpd_bridge_notify(struct device *dev, enum drm_connector_status status);
+#else
+static inline struct device *drm_dp_hpd_bridge_register(struct device *parent,
+ struct device_node *np)
+{
+ return 0;
+}
+
+static inline void drm_aux_hpd_bridge_notify(struct device *dev, enum drm_connector_status status)
+{
+}
+#endif
+
#endif
--
2.39.2
Use the freshly defined DRM_AUX_HPD_BRIDGE instead of open-coding the
same functionality for the DRM bridge chain termination.
Reviewed-by: Bjorn Andersson <[email protected]>
Acked-by: Bjorn Andersson <[email protected]>
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/soc/qcom/Kconfig | 1 +
drivers/soc/qcom/pmic_glink_altmode.c | 33 ++++++++-------------------
2 files changed, 10 insertions(+), 24 deletions(-)
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index b3634e10f6f5..c954001ae79e 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -86,6 +86,7 @@ config QCOM_PMIC_GLINK
depends on OF
select AUXILIARY_BUS
select QCOM_PDR_HELPERS
+ select DRM_AUX_HPD_BRIDGE
help
The Qualcomm PMIC GLINK driver provides access, over GLINK, to the
USB and battery firmware running on one of the coprocessors in
diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
index b78279e2f54c..053b7393e26a 100644
--- a/drivers/soc/qcom/pmic_glink_altmode.c
+++ b/drivers/soc/qcom/pmic_glink_altmode.c
@@ -11,7 +11,7 @@
#include <linux/mutex.h>
#include <linux/property.h>
#include <linux/soc/qcom/pdr.h>
-#include <drm/drm_bridge.h>
+#include <drm/bridge/aux-bridge.h>
#include <linux/usb/typec_altmode.h>
#include <linux/usb/typec_dp.h>
@@ -76,7 +76,7 @@ struct pmic_glink_altmode_port {
struct work_struct work;
- struct drm_bridge bridge;
+ struct device *bridge;
enum typec_orientation orientation;
u16 svid;
@@ -230,10 +230,10 @@ static void pmic_glink_altmode_worker(struct work_struct *work)
else
pmic_glink_altmode_enable_usb(altmode, alt_port);
- if (alt_port->hpd_state)
- drm_bridge_hpd_notify(&alt_port->bridge, connector_status_connected);
- else
- drm_bridge_hpd_notify(&alt_port->bridge, connector_status_disconnected);
+ drm_aux_hpd_bridge_notify(alt_port->bridge,
+ alt_port->hpd_state ?
+ connector_status_connected :
+ connector_status_disconnected);
pmic_glink_altmode_request(altmode, ALTMODE_PAN_ACK, alt_port->index);
};
@@ -365,16 +365,6 @@ static void pmic_glink_altmode_callback(const void *data, size_t len, void *priv
}
}
-static int pmic_glink_altmode_attach(struct drm_bridge *bridge,
- enum drm_bridge_attach_flags flags)
-{
- return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
-}
-
-static const struct drm_bridge_funcs pmic_glink_altmode_bridge_funcs = {
- .attach = pmic_glink_altmode_attach,
-};
-
static void pmic_glink_altmode_put_retimer(void *data)
{
typec_retimer_put(data);
@@ -464,15 +454,10 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev,
alt_port->index = port;
INIT_WORK(&alt_port->work, pmic_glink_altmode_worker);
- alt_port->bridge.funcs = &pmic_glink_altmode_bridge_funcs;
- alt_port->bridge.of_node = to_of_node(fwnode);
- alt_port->bridge.ops = DRM_BRIDGE_OP_HPD;
- alt_port->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
-
- ret = devm_drm_bridge_add(dev, &alt_port->bridge);
- if (ret) {
+ alt_port->bridge = drm_dp_hpd_bridge_register(dev, to_of_node(fwnode));
+ if (IS_ERR(alt_port->bridge)) {
fwnode_handle_put(fwnode);
- return ret;
+ return PTR_ERR(alt_port->bridge);
}
alt_port->dp_alt.svid = USB_TYPEC_DP_SID;
--
2.39.2
Use the freshly defined DRM_AUX_HPD_BRIDGE instead of open-coding the
same functionality for the DRM bridge chain termination.
Acked-by: Bryan O'Donoghue <[email protected]>
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/usb/typec/tcpm/Kconfig | 1 +
drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 41 +++----------------
2 files changed, 7 insertions(+), 35 deletions(-)
diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig
index 0b2993fef564..64d5421c69e6 100644
--- a/drivers/usb/typec/tcpm/Kconfig
+++ b/drivers/usb/typec/tcpm/Kconfig
@@ -80,6 +80,7 @@ config TYPEC_QCOM_PMIC
tristate "Qualcomm PMIC USB Type-C Port Controller Manager driver"
depends on ARCH_QCOM || COMPILE_TEST
depends on DRM || DRM=n
+ select DRM_AUX_HPD_BRIDGE if DRM_BRIDGE
help
A Type-C port and Power Delivery driver which aggregates two
discrete pieces of silicon in the PM8150b PMIC block: the
diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
index 581199d37b49..1a2b4bddaa97 100644
--- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
+++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
@@ -18,7 +18,7 @@
#include <linux/usb/tcpm.h>
#include <linux/usb/typec_mux.h>
-#include <drm/drm_bridge.h>
+#include <drm/bridge/aux-bridge.h>
#include "qcom_pmic_typec_pdphy.h"
#include "qcom_pmic_typec_port.h"
@@ -36,7 +36,6 @@ struct pmic_typec {
struct pmic_typec_port *pmic_typec_port;
bool vbus_enabled;
struct mutex lock; /* VBUS state serialization */
- struct drm_bridge bridge;
};
#define tcpc_to_tcpm(_tcpc_) container_of(_tcpc_, struct pmic_typec, tcpc)
@@ -150,35 +149,6 @@ static int qcom_pmic_typec_init(struct tcpc_dev *tcpc)
return 0;
}
-#if IS_ENABLED(CONFIG_DRM)
-static int qcom_pmic_typec_attach(struct drm_bridge *bridge,
- enum drm_bridge_attach_flags flags)
-{
- return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
-}
-
-static const struct drm_bridge_funcs qcom_pmic_typec_bridge_funcs = {
- .attach = qcom_pmic_typec_attach,
-};
-
-static int qcom_pmic_typec_init_drm(struct pmic_typec *tcpm)
-{
- tcpm->bridge.funcs = &qcom_pmic_typec_bridge_funcs;
-#ifdef CONFIG_OF
- tcpm->bridge.of_node = of_get_child_by_name(tcpm->dev->of_node, "connector");
-#endif
- tcpm->bridge.ops = DRM_BRIDGE_OP_HPD;
- tcpm->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
-
- return devm_drm_bridge_add(tcpm->dev, &tcpm->bridge);
-}
-#else
-static int qcom_pmic_typec_init_drm(struct pmic_typec *tcpm)
-{
- return 0;
-}
-#endif
-
static int qcom_pmic_typec_probe(struct platform_device *pdev)
{
struct pmic_typec *tcpm;
@@ -186,6 +156,7 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev)
struct device_node *np = dev->of_node;
const struct pmic_typec_resources *res;
struct regmap *regmap;
+ struct device *bridge_dev;
u32 base[2];
int ret;
@@ -241,14 +212,14 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev)
mutex_init(&tcpm->lock);
platform_set_drvdata(pdev, tcpm);
- ret = qcom_pmic_typec_init_drm(tcpm);
- if (ret)
- return ret;
-
tcpm->tcpc.fwnode = device_get_named_child_node(tcpm->dev, "connector");
if (!tcpm->tcpc.fwnode)
return -EINVAL;
+ bridge_dev = drm_dp_hpd_bridge_register(tcpm->dev, to_of_node(tcpm->tcpc.fwnode));
+ if (IS_ERR(bridge_dev))
+ return PTR_ERR(bridge_dev);
+
tcpm->tcpm_port = tcpm_register_port(tcpm->dev, &tcpm->tcpc);
if (IS_ERR(tcpm->tcpm_port)) {
ret = PTR_ERR(tcpm->tcpm_port);
--
2.39.2
On Sun, Dec 03, 2023 at 02:43:27PM +0300, Dmitry Baryshkov wrote:
> Greg, could you please ack the last patch to be merged through the
> drm-misc tree? You have acked patch 3, but since that time I've added
> patches 4-6.
That is up to the typec maintainer to ack, not me!
thanks,
greg k-h
On Mon, 4 Dec 2023 at 14:21, Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Sun, Dec 03, 2023 at 02:43:27PM +0300, Dmitry Baryshkov wrote:
> > Greg, could you please ack the last patch to be merged through the
> > drm-misc tree? You have acked patch 3, but since that time I've added
> > patches 4-6.
>
> That is up to the typec maintainer to ack, not me!
Hmm, true. I didn't notice supporter vs maintainer.
Heikki, then we should bug you about that patch.
--
With best wishes
Dmitry
Hi Dmitry,
On Sun, Dec 03, 2023 at 02:43:33PM +0300, Dmitry Baryshkov wrote:
> Use the freshly defined DRM_AUX_HPD_BRIDGE instead of open-coding the
> same functionality for the DRM bridge chain termination.
>
> Acked-by: Bryan O'Donoghue <[email protected]>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
I'm sorry, I've completely missed this second typec patch.
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> drivers/usb/typec/tcpm/Kconfig | 1 +
> drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 41 +++----------------
> 2 files changed, 7 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig
> index 0b2993fef564..64d5421c69e6 100644
> --- a/drivers/usb/typec/tcpm/Kconfig
> +++ b/drivers/usb/typec/tcpm/Kconfig
> @@ -80,6 +80,7 @@ config TYPEC_QCOM_PMIC
> tristate "Qualcomm PMIC USB Type-C Port Controller Manager driver"
> depends on ARCH_QCOM || COMPILE_TEST
> depends on DRM || DRM=n
> + select DRM_AUX_HPD_BRIDGE if DRM_BRIDGE
> help
> A Type-C port and Power Delivery driver which aggregates two
> discrete pieces of silicon in the PM8150b PMIC block: the
> diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
> index 581199d37b49..1a2b4bddaa97 100644
> --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
> +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
> @@ -18,7 +18,7 @@
> #include <linux/usb/tcpm.h>
> #include <linux/usb/typec_mux.h>
>
> -#include <drm/drm_bridge.h>
> +#include <drm/bridge/aux-bridge.h>
>
> #include "qcom_pmic_typec_pdphy.h"
> #include "qcom_pmic_typec_port.h"
> @@ -36,7 +36,6 @@ struct pmic_typec {
> struct pmic_typec_port *pmic_typec_port;
> bool vbus_enabled;
> struct mutex lock; /* VBUS state serialization */
> - struct drm_bridge bridge;
> };
>
> #define tcpc_to_tcpm(_tcpc_) container_of(_tcpc_, struct pmic_typec, tcpc)
> @@ -150,35 +149,6 @@ static int qcom_pmic_typec_init(struct tcpc_dev *tcpc)
> return 0;
> }
>
> -#if IS_ENABLED(CONFIG_DRM)
> -static int qcom_pmic_typec_attach(struct drm_bridge *bridge,
> - enum drm_bridge_attach_flags flags)
> -{
> - return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
> -}
> -
> -static const struct drm_bridge_funcs qcom_pmic_typec_bridge_funcs = {
> - .attach = qcom_pmic_typec_attach,
> -};
> -
> -static int qcom_pmic_typec_init_drm(struct pmic_typec *tcpm)
> -{
> - tcpm->bridge.funcs = &qcom_pmic_typec_bridge_funcs;
> -#ifdef CONFIG_OF
> - tcpm->bridge.of_node = of_get_child_by_name(tcpm->dev->of_node, "connector");
> -#endif
> - tcpm->bridge.ops = DRM_BRIDGE_OP_HPD;
> - tcpm->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
> -
> - return devm_drm_bridge_add(tcpm->dev, &tcpm->bridge);
> -}
> -#else
> -static int qcom_pmic_typec_init_drm(struct pmic_typec *tcpm)
> -{
> - return 0;
> -}
> -#endif
> -
> static int qcom_pmic_typec_probe(struct platform_device *pdev)
> {
> struct pmic_typec *tcpm;
> @@ -186,6 +156,7 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev)
> struct device_node *np = dev->of_node;
> const struct pmic_typec_resources *res;
> struct regmap *regmap;
> + struct device *bridge_dev;
> u32 base[2];
> int ret;
>
> @@ -241,14 +212,14 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev)
> mutex_init(&tcpm->lock);
> platform_set_drvdata(pdev, tcpm);
>
> - ret = qcom_pmic_typec_init_drm(tcpm);
> - if (ret)
> - return ret;
> -
> tcpm->tcpc.fwnode = device_get_named_child_node(tcpm->dev, "connector");
> if (!tcpm->tcpc.fwnode)
> return -EINVAL;
>
> + bridge_dev = drm_dp_hpd_bridge_register(tcpm->dev, to_of_node(tcpm->tcpc.fwnode));
> + if (IS_ERR(bridge_dev))
> + return PTR_ERR(bridge_dev);
> +
> tcpm->tcpm_port = tcpm_register_port(tcpm->dev, &tcpm->tcpc);
> if (IS_ERR(tcpm->tcpm_port)) {
> ret = PTR_ERR(tcpm->tcpm_port);
> --
> 2.39.2
--
heikki
On Sun, 03 Dec 2023 14:43:27 +0300, Dmitry Baryshkov wrote:
> Greg, could you please ack the last patch to be merged through the
> drm-misc tree? You have acked patch 3, but since that time I've added
> patches 4-6.
>
> Supporting DP/USB-C can result in a chain of several transparent
> bridges (PHY, redrivers, mux, etc). All attempts to implement DP support
> in a different way resulted either in series of hacks or in device tree
> not reflecting the actual hardware design. This results in drivers
> having similar boilerplate code for such bridges.
>
> [...]
Applied to drm-misc-next, thanks!
[1/6] drm/bridge: add transparent bridge helper
commit: 2a04739139b2b2761571e18937e2400e71eff664
[2/6] phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE
commit: 35921910bbd0b6ab595cead16d0c8faadbf2fd94
[3/6] usb: typec: nb7vpq904m: switch to DRM_AUX_BRIDGE
commit: c5d296bad640b190c52ef7508114d70e971a4bba
[4/6] drm/bridge: implement generic DP HPD bridge
commit: e560518a6c2e60f1566473c146fddcff3281f617
[5/6] soc: qcom: pmic-glink: switch to DRM_AUX_HPD_BRIDGE
commit: 2bcca96abfbf89d26fc10fc92e40532bb2ae8891
[6/6] usb: typec: qcom-pmic-typec: switch to DRM_AUX_HPD_BRIDGE
commit: 7d9f1b72b29698e3030c2b163522cf4aa91b47e9
Best regards,
--
Dmitry Baryshkov <[email protected]>