[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <faada5fc-3dda-2d69-508a-15dbf5ddbbcc@linaro.org>
Date: Thu, 27 Apr 2023 21:00:33 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: neil.armstrong@...aro.org,
Bryan O'Donoghue <pure.logic@...us-software.ie>,
Bjorn Andersson <quic_bjorande@...cinc.com>
Cc: Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>,
Johan Hovold <johan@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/7] phy: qcom-qmp-combo: Introduce drm_bridge
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@...cinc.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@...cinc.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@...aro.org>
>
> Neil
>
>
>>
>> ---
>> bod
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists