| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <08eb7560-c13e-462f-8110-d4ce5ccbd687@rock-chips.com>
Date: Mon, 13 Oct 2025 09:26:06 +0800
From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
Chaoyi Chen <kernel@...kyi.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>, Heiko Stuebner
<heiko@...ech.de>, Sandy Huang <hjc@...k-chips.com>,
Andy Yan <andy.yan@...k-chips.com>,
Yubing Zhang <yubing.zhang@...k-chips.com>,
Frank Wang <frank.wang@...k-chips.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Amit Sunil Dhamne <amitsd@...gle.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Dragan Simic <dsimic@...jaro.org>, Johan Jonker <jbx6244@...il.com>,
Diederik de Haas <didi.debian@...ow.org>,
Peter Robinson <pbrobinson@...il.com>, linux-usb@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-phy@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v5 6/8] drm/rockchip: cdn-dp: Add multiple bridges to
support PHY port selection
On 10/12/2025 2:52 AM, Dmitry Baryshkov wrote:
> On Sat, Oct 11, 2025 at 11:32:31AM +0800, Chaoyi Chen wrote:
>> From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
>>
>> The RK3399 has two USB/DP combo PHY and one CDN-DP controller. And
>> the CDN-DP can be switched to output to one of the PHYs. If both ports
>> are plugged into DP, DP will select the first port for output.
>>
>> This patch adds support for multiple bridges, enabling users to flexibly
>> select the output port. For each PHY port, a separate encoder and bridge
>> are registered.
>>
>> The change is based on the DRM AUX HPD bridge, rather than the
>> extcon approach. This requires the DT to correctly describe the
>> connections between the first bridge in bridge chain and DP
>> controller. For example, the bridge chain may be like this:
>>
>> PHY aux birdge -> fsa4480 analog audio switch bridge ->
>> onnn,nb7vpq904m USB reminder bridge -> USB-C controller AUX HPD bridge
>>
>> In this case, the connection relationships among the PHY aux bridge
>> and the DP contorller need to be described in DT.
>>
>> In addition, the cdn_dp_parse_hpd_bridge_dt() will parses it and
>> determines whether to register one or two bridges.
>>
>> Since there is only one DP controller, only one of the PHY ports can
>> output at a time. The key is how to switch between different PHYs,
>> which is handled by cdn_dp_switch_port() and cdn_dp_enable().
>>
>> There are two cases:
>>
>> 1. Neither bridge is enabled. In this case, both bridges can
>> independently read the EDID, and the PHY port may switch before
>> reading the EDID.
>>
>> 2. One bridge is already enabled. In this case, other bridges are not
>> allowed to read the EDID. So we will try to return the cached EDID.
>>
>> Since the scenario of two ports plug in at the same time is rare,
>> I don't have a board which support two TypeC connector to test this.
>> Therefore, I tested forced switching on a single PHY port, as well as
>> output using a fake PHY port alongside a real PHY port.
>>
>> Signed-off-by: Chaoyi Chen <chaoyi.chen@...k-chips.com>
>> ---
>>
>> Changes in v5:
>> - By parsing the HPD bridge chain, set the connector's of_node to the
>> of_node corresponding to the USB-C connector.
>> - Return EDID cache when other port is already enabled.
>>
>> drivers/gpu/drm/rockchip/Kconfig | 2 +
>> drivers/gpu/drm/rockchip/cdn-dp-core.c | 355 +++++++++++++++++++++----
>> drivers/gpu/drm/rockchip/cdn-dp-core.h | 24 +-
>> 3 files changed, 324 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
>> index faf50d872be3..7472ec923cfd 100644
>> --- a/drivers/gpu/drm/rockchip/Kconfig
>> +++ b/drivers/gpu/drm/rockchip/Kconfig
>> @@ -55,6 +55,8 @@ config ROCKCHIP_CDN_DP
>> select DRM_DISPLAY_HELPER
>> select DRM_BRIDGE_CONNECTOR
>> select DRM_DISPLAY_DP_HELPER
>> + select DRM_AUX_BRIDGE
>> + select DRM_AUX_HPD_BRIDGE
> You are not using them in this driver, so this is not correct. Please
> declare Kconfig dependencies for the drivers that actually call the API,
> otherwise the LKP or somebody else can get compile errors because this
> driver wasn't selected.
Thanks for the clarification. Will fix in v6.
>
>> help
>> This selects support for Rockchip SoC specific extensions
>> for the cdn DP driver. If you want to enable Dp on
> [...]
>
>> +
>> + /*
>> + *
>> + * &dp_out {
>> + * dp_controller_output0: endpoint@0 {
>> + * remote-endpoint = <&dp_phy0_in>
>> + * };
>> + *
>> + * dp_controller_output1: endpoint@1 {
>> + * remote-endpoint = <&dp_phy1_in>
>> + * };
>> + * };
>> + *
>> + * &tcphy0_dp {
>> + * port {
>> + * tcphy0_typec_dp: endpoint@0 {
>> + * reg = <0>;
>> + * remote-endpoint = <&usbc0_dp>;
>> + * };
>> + *
>> + * dp_phy0_in: endpoint@1 {
>> + * reg = <1>;
>> + * remote-endpoint = <&dp_controller_output0>;
>> + * };
>> + * };
>> + * };
>> + *
>> + * &tcphy1_dp {
>> + * ...
>> + * };
>> + *
>> + */
>> +
>> + /* One endpoint may correspond to one HPD bridge. */
>> + for_each_of_graph_port_endpoint(port, dp_ep) {
>> + struct device_node *phy_bridge_node __free(device_node) =
>> + of_graph_get_remote_port_parent(dp_ep);
>> +
>> + bridge = of_drm_find_bridge(phy_bridge_node);
>> + if (!bridge) {
>> + ret = -EPROBE_DEFER;
>> + goto out;
>> + }
>> +
>> + dp->hpd_bridge_valid = true;
>> + dp->hpd_bridge_list[count].bridge = bridge;
>> + dp->hpd_bridge_list[count].parent = dp;
>> + dp->hpd_bridge_list[count].id = count;
> This looks misnamed. They are not necessarily HPD bridges. There can be
> a random chain between your controller and the actual output / connector
> /etc.
Yes, and more precisely, this should be `pervious_bridge_list` . Will fix in v6.
>
>> + count++;
>> + }
>> +
>> +out:
>> + dp->bridge_count = count ? count : 1;
>> + return ret;
>> +}
>> +
>> +static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>> +{
>> + struct cdn_dp_device *dp = dev_get_drvdata(dev);
>> + struct drm_bridge *bridge, *hpd_bridge;
>> + struct drm_encoder *encoder;
>> + struct cdn_dp_port *port;
>> + struct drm_device *drm_dev = data;
>> + struct cdn_dp_bridge *bridge_list;
> Why is it bridge_list?
Maybe it's still a naming issue. There may be multiple struct cdn_dp_bridge that will eventually be added to a list. It might be better to rename it to `dp_bridge`
>
>> + int ret, i;
>> +
>> + ret = cdn_dp_parse_dt(dp);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = cdn_dp_parse_hpd_bridge_dt(dp);
>> + if (ret)
>> + return ret;
>> +
>> + dp->drm_dev = drm_dev;
>> + dp->connected = false;
>> + dp->active = false;
>> + dp->active_port = -1;
>> + dp->fw_loaded = false;
>> +
>> + for (i = 0; i < dp->bridge_count; i++) {
>> + bridge_list = devm_drm_bridge_alloc(dev, struct cdn_dp_bridge, bridge,
>> + &cdn_dp_bridge_funcs);
>> + if (IS_ERR(bridge_list))
>> + return PTR_ERR(bridge_list);
>> + bridge_list->id = i;
>> + bridge_list->parent = dp;
>> + if (!dp->hpd_bridge_valid)
>> + bridge_list->connected = true;
>> + dp->bridge_list[i] = bridge_list;
>> + }
>> +
--
Best,
Chaoyi
Powered by blists - more mailing lists