Hi Dmitry, Thank you for the patches. On Thu, Aug 17, 2023 at 05:55:13PM +0300, Dmitry Baryshkov wrote: > Supporting DP/USB-C can result in a chain of several transparent > bridges (PHY, redrivers, mux, etc). This results in drivers having > similar boilerplate code for such bridges. What do you mean by transparent bridge here ? Bridges are a DRM concept, and as far as I can tell, a PHY isn't a bridge. Why does it need to be handled as one, especially if it's completely transparent ? > 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. Can't device links help avoiding defer probing in those cases ? > Last, but not least, this results in the the internal knowledge of DRM > subsystem slowly diffusing into other subsystems, like PHY or USB/TYPEC. Why so ? The PHY subsystem should provide a PHY, without considering what subsystem it will be used by. This patch series seems to me to actually create this DRM dependency in other subsystems, which I don't think is a very good idea. Resources should be registered in their own subsystem with the appropriate API, not in a way that is tied to a particular consumer. > 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. I'm not thrilled :-( Let's discuss the questions above first. > Proposed merge strategy: immutable branch with the drm commit, which is > then merged into PHY and USB subsystems together with the corresponding > patch. > > 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 (3): > drm/bridge: add transparent bridge helper > phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE > usb: typec: nb7vpq904m: switch to DRM_AUX_BRIDGE > > drivers/gpu/drm/bridge/Kconfig | 9 ++ > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/aux-bridge.c | 132 ++++++++++++++++++++++ > drivers/phy/qualcomm/Kconfig | 2 +- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 44 +------- > drivers/usb/typec/mux/Kconfig | 2 +- > drivers/usb/typec/mux/nb7vpq904m.c | 44 +------- > include/drm/bridge/aux-bridge.h | 19 ++++ > 8 files changed, 167 insertions(+), 86 deletions(-) > create mode 100644 drivers/gpu/drm/bridge/aux-bridge.c > create mode 100644 include/drm/bridge/aux-bridge.h -- Regards, Laurent Pinchart
