Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 27 Apr 2023 21:00:33 +0300 | Subject | Re: [PATCH 5/7] phy: qcom-qmp-combo: Introduce drm_bridge | From | Dmitry Baryshkov <> |
| |
On 27/04/2023 16:11, Neil Armstrong wrote: > On 26/04/2023 12:33, Bryan O'Donoghue wrote: >> On Tue, Apr 25, 2023 at 4:40 AM Bjorn Andersson >> <quic_bjorande@quicinc.com> wrote: >>> >>> The QMP combo PHY sits in an of_graph connected between the DisplayPort >>> controller and a USB Type-C connector (or possibly a redriver). >>> >>> The TCPM needs to be able to convey the HPD signal to the DisplayPort >>> controller, but no directly link is provided by DeviceTree so the signal >>> needs to "pass through" the QMP combo phy. >>> >>> Handle this by introducing a drm_bridge which upon initialization finds >>> the next bridge (i.e. the usb-c-connector) and chain this together. This >>> way HPD changes in the connector will propagate to the DisplayPort >>> driver. >>> >>> The connector bridge is resolved lazily, as the TCPM is expected to be >>> able to resolve the typec mux and switch at probe time, so the QMP combo >>> phy will probe before the TCPM. >>> >>> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> >>> --- >>> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 36 +++++++++++++++++++++++ >>> 1 file changed, 36 insertions(+) >>> >>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> index 5d6d6ef3944b..84bc08002537 100644 >>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >>> @@ -22,6 +22,8 @@ >>> #include <linux/usb/typec.h> >>> #include <linux/usb/typec_mux.h> >>> >>> +#include <drm/drm_bridge.h> >>> + >>> #include <dt-bindings/phy/phy-qcom-qmp.h> >>> >>> #include "phy-qcom-qmp.h" >>> @@ -1332,6 +1334,8 @@ 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; >>> }; >>> @@ -3196,6 +3200,34 @@ static int qmp_combo_register_clocks(struct >>> qmp_combo *qmp, struct device_node * >>> return devm_add_action_or_reset(qmp->dev, >>> phy_clk_release_provider, dp_np); >>> } >>> >>> +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)) >>> + return dev_err_probe(qmp->dev, PTR_ERR(next_bridge), >>> "failed to acquire drm_bridge\n"); >>> + >>> + 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); >>> +} >>> + >>> static int qmp_combo_parse_dt_lecacy_dp(struct qmp_combo *qmp, >>> struct device_node *np) >>> { >>> struct device *dev = qmp->dev; >>> @@ -3459,6 +3491,10 @@ static int qmp_combo_probe(struct >>> platform_device *pdev) >>> if (ret) >>> return ret; >>> >>> + ret = qmp_combo_dp_register_bridge(qmp); >>> + if (ret) >>> + return ret; > > I think the DRM part should be only built if CONFIG_DRM is enabled, I don't > have a strong opinion on this, I think Vinod could help here. > >>> + >>> /* Check for legacy binding with child nodes. */ >>> usb_np = of_get_child_by_name(dev->of_node, "usb3-phy"); >>> if (usb_np) { >>> -- >>> 2.39.2 >>> >> >> You need to add some or all of these >> select DRM_DISPLAY_DP_HELPER >> select DRM_DISPLAY_HELPER >> select DRM_DP_AUX_BUS >> select DRM_KMS_HELPER >> select DRM_MIPI_DSI >> select DRM_PANEL >> >> >> /opt/linaro/gcc-linaro-7.5.0-2019.12-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-ld: >> Unexpected GOT/PLT entries detected! >> /opt/linaro/gcc-linaro-7.5.0-2019.12-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-ld: >> Unexpected run-time procedure linkages detected! >> drivers/phy/qualcomm/phy-qcom-qmp-combo.o: In function >> `qmp_combo_bridge_attach': >> phy-qcom-qmp-combo.c:(.text+0xb50): undefined reference to >> `devm_drm_of_get_bridge' >> phy-qcom-qmp-combo.c:(.text+0xb6c): undefined reference to >> `drm_bridge_attach' >> drivers/phy/qualcomm/phy-qcom-qmp-combo.o: In function `qmp_combo_probe': >> phy-qcom-qmp-combo.c:(.text+0x13fc): undefined reference to >> `devm_drm_bridge_add' > > I think CONFIG_DRM_PANEL_BRIDGE in addition to CONFIG_DRM. should be > enough. >
I'd say DRM_PANEL_BRIDGE || !DRM_PANEL_BRIDGE in addition to DRM. And we probably should fix the devm_drm_of_get_bridge() stub to use drm_of_find_panel_or_bridge() with panel = NULL.
> With this config added and my drm-bridge hat: > > Acked-by: Neil Armstrong <neil.armstrong@linaro.org> > > Neil > > >> >> --- >> bod >
-- With best wishes Dmitry
| ![\](/images/icornerr.gif) |